tdunning / open-json

Open JSON - a truly open source JSON implementation
Apache License 2.0
18 stars 18 forks source link

Each JSONObject constructor should only throw runtime exceptions #10

Closed klopfdreh closed 7 years ago

klopfdreh commented 7 years ago

Hi @tdunning ,

@bitstorm mentioned a good point we noticed during some code testings against the original json.org and the open-json implementation:

According to the API of json.org every constructor only throws runtime exceptions. If we change that behavior the user is required to catch those.

I think it would be better both implementation behave the same way.

Because Apache Wicket 8.0 is going to be released soon it would be great if you could release version 1.8 (with this change) so that we only have to change the dependency for the release.

Thank you so much for you time!

klopfdreh commented 7 years ago

ping @tdunning

tdunning commented 7 years ago

Thanks for the ping.

Yeah... I think your suggestion is a good one.

Related, what do you think about the JSONString interface that seems to do the same thing as the special purpose toString hack we put in a while ago? Should we just let sleeping dogs lie?

Also, what are your plans for moving to a modern (and properly licensed) JSON alternative like Jackson?

klopfdreh commented 7 years ago

Hi @tdunning ,

from what I remember JSONString was reimplemented by someone, because otherwise there were issues with some existing modules.

I would suggest to not change the API dramatically by removing interfaces, because open-json is used as a replacement for json.org.

I think in some further versions of Wicket we might consider to switch to Jackson, but for now we wanted to keep the migration steps as small as possible.

So in my opinion open-json does a great job 😃

tdunning commented 7 years ago

I didn't plan to remove any interfaces. Just wondering if you guys wanted to join forces with those using the method supported by the json.org folks. No big deal either way.

Any thoughts about the progress on sunsetting the use of Open Json?

On Thu, Jan 19, 2017 at 11:15 AM, Tobias Soloschenko < notifications@github.com> wrote:

Hi @tdunning https://github.com/tdunning ,

from what I remember JSONString was reimplemented by someone, because otherwise there were issues with some existing modules.

I would suggest to not change the API dramatically by removing interfaces

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tdunning/open-json/pull/10#issuecomment-273870187, or mute the thread https://github.com/notifications/unsubscribe-auth/AAPSejcfST703kY6PcSJkDHWMkAl_ELpks5rT7ZbgaJpZM4Lj7Kv .

klopfdreh commented 7 years ago

Not yet, but I think this requires some discussion with other devs of wicket. I also could imagine that because it is a so lightweight implementation to only do some json rendering that open-json will be used further on.

klopfdreh commented 7 years ago

Hi @tdunning ,

I just tried out to fetch the newest version, but it seems that it is not on the cmr yet:

https://search.maven.org/#search%7Cga%7C1%7Cg%3A%22com.tdunning%22

tdunning commented 7 years ago

Correct. I got diverted by real-life (tm)

On Fri, Jan 20, 2017 at 12:11 AM, Tobias Soloschenko < notifications@github.com> wrote:

Hi @tdunning https://github.com/tdunning ,

I just tried out to fetch the newest version, but it seems that it is not on the cmr yet:

https://search.maven.org/#search%7Cga%7C1%7Cg%3A%22com.tdunning%22

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tdunning/open-json/pull/10#issuecomment-274004465, or mute the thread https://github.com/notifications/unsubscribe-auth/AAPSesVLPyvf_u0Parars6MTEzGEEVp1ks5rUGxKgaJpZM4Lj7Kv .

tdunning commented 7 years ago

Finished the release. The artifacts should hit central over the next few hours.

Sorry for the delay, thanks for the ping.

On Fri, Jan 20, 2017 at 10:08 AM, Ted Dunning ted.dunning@gmail.com wrote:

Correct. I got diverted by real-life (tm)

On Fri, Jan 20, 2017 at 12:11 AM, Tobias Soloschenko < notifications@github.com> wrote:

Hi @tdunning https://github.com/tdunning ,

I just tried out to fetch the newest version, but it seems that it is not on the cmr yet:

https://search.maven.org/#search%7Cga%7C1%7Cg%3A%22com.tdunning%22

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tdunning/open-json/pull/10#issuecomment-274004465, or mute the thread https://github.com/notifications/unsubscribe-auth/AAPSesVLPyvf_u0Parars6MTEzGEEVp1ks5rUGxKgaJpZM4Lj7Kv .

tdunning commented 7 years ago

The artifacts are on Maven central now.

https://repo1.maven.org/maven2/com/tdunning/json/1.8/

I verified also with my test program after nuking that part of my maven local cache repo.

On Fri, Jan 20, 2017 at 10:14 AM, Ted Dunning ted.dunning@gmail.com wrote:

Finished the release. The artifacts should hit central over the next few hours.

Sorry for the delay, thanks for the ping.

On Fri, Jan 20, 2017 at 10:08 AM, Ted Dunning ted.dunning@gmail.com wrote:

Correct. I got diverted by real-life (tm)

On Fri, Jan 20, 2017 at 12:11 AM, Tobias Soloschenko < notifications@github.com> wrote:

Hi @tdunning https://github.com/tdunning ,

I just tried out to fetch the newest version, but it seems that it is not on the cmr yet:

https://search.maven.org/#search%7Cga%7C1%7Cg%3A%22com.tdunning%22

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tdunning/open-json/pull/10#issuecomment-274004465, or mute the thread https://github.com/notifications/unsubscribe-auth/AAPSesVLPyvf_u0Parars6MTEzGEEVp1ks5rUGxKgaJpZM4Lj7Kv .

klopfdreh commented 7 years ago

Thank you! I'm going to change the version in the Wicket POM, soon.