Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make NumberSerializers.Base public and its inherited classes not final #2116

Closed
edouardmercier opened this issue Aug 20, 2018 · 4 comments
Closed
Milestone

Comments

@edouardmercier
Copy link

In v2.9.6, I needed to have the NumberSerializers.FloatSerializer customized so as to remove the trailing float 0 after the decimal separator when the value is actually an integer, because I could not find a way to configure the Jackson serializer so. Example: I want the "1.0" value to be serialized as "1" and not "1.0".

Hence, I wanted to inherit from the NumberSerializers.FloatSerializer class, so as to overload the JsonSerializer.serialize(Object value, JsonGenerator generator, SerializerProvider provider) method, but discovered that:

  1. the NumberSerializers.Base class is protected ;
  2. the NumberSerializers.FloatSerializer is final.

Would it be possible to make the NumberSerializers.Base class public and the NumberSerializers.FloatSerializer not final, please?

Indeed, this would make the number serializer more open and prevent from copy/pasting the NumberSerializers.Base code when writing a number serializer which overloads the default behavior.

@cowtowncoder
Copy link
Member

This is possible to do for 2.10. I am bit torn on whether allowing extension by sub-classing is a good thing or not (ideally it should not be used), partly because it will then further limit implementation aspects of standard (de)serializers as internal methods become part of semi-public API.

On NumberSerializers.Base: is there particular need for this? As in, specific immediate problem, or just general wish that this would allow easier implementation of support for other numeric types?
Since it is geared towards support for JDK primitive/wrapper types (... although I guess it may also support BigDecimal / BigInteger? it has been a while since I looked into this) it is not necessarily designed for extensibility at this point.

@edouardmercier
Copy link
Author

I understand your point regarding the default serialization behavior in Java and about the fact that opening too much the code sometimes leads to bad usage.

Maybe subclassing is not the right choice and that you may do this by delegation.

Regarding NumberSerializers.Base, you are right: if NumberSerializers.FloatSerializer is not final anymore, then I will be able to override the JsonSerializer.serialize(Object value, JsonGenerator generator, SerializerProvider provider) method, hence you do not need to have it public.

In my opinion, an API like Jackson should have a descent behavior — which is the case — but should be enough open so that you may change this behavior, at your own risks. Yes, developers may override a class in a wrong way, but if you have the right documentation along with the right warning, you will have done all you can do so that they have all the material to do the right thing.

@cowtowncoder
Copy link
Member

Perhaps I should just add a note in Javadoc stating that while extension is possible, one should be aware that this is not an "official" extension point -- that is, it may work but may be subject to more changes than public API. That is, "only use if you know what you are doing".
This is along my philosophy as well: while it is good to prevent completely wrong usage, support right ways, there are many grey areas in which usage sometimes makes sense. Especially if there are no better mechanisms.

@edouardmercier
Copy link
Author

I'm fine with your proposal, I think that this is the right way of doing this. Good news.

@cowtowncoder cowtowncoder added this to the 2.10.0 milestone Aug 28, 2018
cowtowncoder added a commit that referenced this issue Aug 28, 2018
@cowtowncoder cowtowncoder removed the 2.10 label Apr 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants