discomarathon / google-gson

Automatically exported from code.google.com/p/google-gson
0 stars 0 forks source link

JsonNull.createJsonNull should be public, constructor should be private #97

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Version: gson-1.3 from http://google-gson.googlecode.com/svn/mavenrepo

I assume it's not public because the original intention was to only need it
in automatically gson serialization. However I'm writing a serializer and
would like to return json null programmatically when desired (say, to
handle GsonBuilder#serializeNulls).

Original issue reported on code.google.com by estebis...@gmail.com on 28 Jan 2009 at 4:51

GoogleCodeExporter commented 9 years ago
Any reason why you will not prefer to use just new JsonNull()?

Original comment by inder123 on 28 Jan 2009 at 5:02

GoogleCodeExporter commented 9 years ago
No, that's what I'm doing now, but why restrict the singleton to internal 
usage? Why
then allow new instance and dilute the meaning of the singleton? As it stands 
now, on
deserialization, one would have to test (jsonElem instanceof JsonNull) instead 
of
(JsonNull.getInstance().equals(jsonElem)).

Original comment by estebis...@gmail.com on 28 Jan 2009 at 6:14

GoogleCodeExporter commented 9 years ago
We certainly can not make the constructor private anymore since that will break 
backwards compatibility. Also, all instance of JsonNull are equal to each other 
so 
you can just use equals(). 

We (Joel and I, the creators of this library) had lots of discussions on making 
the 
factory method public, and deprecate the constructor. We sort-of concluded that 
new 
JsonNull() is more aligned with how other JsonElements are created so was 
slightly 
preferred. The performance overhead is probably not all that significant. So, I 
wanted to get some external input, and hence I am trying to understand your 
perspective.

Original comment by inder123 on 29 Jan 2009 at 2:20

GoogleCodeExporter commented 9 years ago
I see. Fair enough. No, it isn't a performance concern. That would be premature 
to
say the least. It was more of an inconsistency that stood out for me (enough to 
say
something I guess). Somehow I saw it and thought for a moment that I should be 
using
it (because it returned a singleton, usually a very proactive mechanism by a 
class
author).

Assuming that new JsonNull is the intended way going forward, it might not hurt 
to
put a note to this effect on the createJsonNull method, perhaps even deprecate 
it.

All that said, we can end this issue as "wontfix".

Original comment by estebis...@gmail.com on 29 Jan 2009 at 2:31

GoogleCodeExporter commented 9 years ago
We will add the comment. 

Original comment by inder123 on 29 Jan 2009 at 2:38