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

Implement an instance toString() method on JSONObject #4219

Merged
merged 1 commit into from Apr 21, 2015

Conversation

alokmenghrajani
Copy link
Contributor

http://www.jooq.org/javadoc/latest/ is not very clear about JSONObject
taking a map as a parameter in the constructor but calling toString on
the resulting object won't return a JSON string.

I feel things can be improved by either adding something to the documentation
or by having a toString() method which calls toJSONString. This commit
implements the latter.

http://www.jooq.org/javadoc/latest/ is not very clear about JSONObject
taking a map as a parameter in the constructor but calling toString on
the resulting object won't return a JSON string.

I feel things can be improved by either adding something to the documentation
or by having a toString() method which calls toJSONString. This commit
implements the latter.
@lukaseder
Copy link
Member

Hmm, interesting. Thanks for the contribution!

But instead of improving our version of the library, we might just upgrade to the latest version of JSONSimple, which already includes this change:
https://github.com/fangyidong/json-simple/blob/tag_release_1_1_1/src/main/java/org/json/simple/JSONObject.java

(discussions about embedding dependencies can be found here, for instance: #3869)

@lukaseder
Copy link
Member

Upgrading JSONSimple broke our integration tests, so we cannot do that. It might be a good time to think about replacing the dependency entirely by the standard jsonp in jOOQ 4.0 (#4220).

I'll merge your pull request as it adds immediate value without any risks. The merge can be done without signing our transfer of rights agreement, as this third-party section of the code is ASL 2.0 only licensed.

Thanks again for this contribution!

lukaseder added a commit that referenced this pull request Apr 21, 2015
[#4219] Implement an instance toString() method on JSONObject
@lukaseder lukaseder merged commit ef51add into jOOQ:master Apr 21, 2015
@lukaseder lukaseder changed the title Implements an instance toString() method on JSONObject. Implement an instance toString() method on JSONObject Apr 21, 2015
@alokmenghrajani
Copy link
Contributor Author

👍
Do you think JSONArray should also get the same change?

@alokmenghrajani alokmenghrajani deleted the alok/JSONObject branch April 21, 2015 17:03
@lukaseder
Copy link
Member

Yes, good idea. I didn't even check that

lukaseder added a commit that referenced this pull request Apr 22, 2015
@lukaseder
Copy link
Member

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants