irmen / Pyrolite

Java and .NET client interface for Pyro5 protocol
MIT License
178 stars 47 forks source link

Allowing Subclassing of Pickler #40

Closed bensusman closed 8 years ago

bensusman commented 8 years ago

Adding to allow for custom pickle implementation (working on implemeting a lighter-weight streaming version of pickle)

Simply making private variables protected to allow for subclasses.

irmen commented 8 years ago

Can you add some clues as to why you need direct access to these private fields? Can't you do it via virtual methods?

bensusman commented 8 years ago

In general, I found I could not effectively subclass Pickle without access to these fields.

For example, it is not apparent to me how I can effectively set the private OutputStream out such that when calling the public void save(Object o) method will not result in an Exception.

Is your suggestion to expose these fields as getters/setters rather than making the fields protected or did you have something else in mind?

irmen commented 8 years ago

I was thinking you could perhaps simply override the save method. Also, are you aware of the registerCustomPickler method? I would think that this allows you to register a special kind of pickler that streams the objects into the pickle?

bensusman commented 8 years ago

Thank you for the comment.

I think there is a basic issue with trying to override the save method: It appears to me not possible to subsequently use a bulk of the already defined methods / implementation.

For example, if I override save:

In essence, I would essentially be copying the Pickler, but really only need to override the dump method or implement a similar method to dump and have the variables be protected.

irmen commented 8 years ago

And registering a custom pickling class is not enough to achieve your goal either?

I don't really mind to make things public (though for abstraction's sake I'd rather keep stuff private as much as possible) but it's a bit hard to see what you need to do in your situation without seeing the actual code you're writing.

Would it be possible and useful to share your version of the pickler once it's finished?

bensusman commented 8 years ago

It seems that registering a custom pickling class is not enough because we want to be able to separate the objects in a streaming scenario.

In this case, note I am only suggesting that some of this fields become protected to allow for subclassing, not to lift these to public.

I can send you an anonymized code snippet if that would help in understanding the use case.

Thanks. Ben

irmen commented 8 years ago

While not being used, I've made similar changes to the .net version of the pickler. I'd like them to remain in sync.

bensusman commented 8 years ago

Thank you.

Would it be possible to create a release with these changes?

irmen commented 8 years ago

I suppose so, are the current changes enough for your cause?

bensusman commented 8 years ago

Yes . The current changes seem to be enough for what we are working on. Thank you.

irmen commented 8 years ago

Release 4.13 has been uploaded to Nuget and Maven.