Skip to content

Conversation

@joeferner
Copy link
Contributor

No description provided.

@stleary
Copy link
Owner

stleary commented Feb 27, 2016

Seems like a reasonable enhancement.

Should the methods be renamed as getters, e.g. getMap() instead of toMap() ?
Should the return values be Map and List<?> ?

Otherwise, no objections here.
Any other opinions?

@johnjaylward
Copy link
Contributor

I think the toMap and toList are reasonable names as they imply a
conversion, which is what the implementation is doing.
On Feb 27, 2016 11:05 AM, "Sean Leary" notifications@github.com wrote:

Seems like a reasonable enhancement.

Should the methods be renamed as getters, e.g. getMap() instead of toMap()
?
Should the return values be Map and List<?> ?

Otherwise, no objections here.
Any other opinions?


Reply to this email directly or view it on GitHub
#203 (comment).

@johnjaylward
Copy link
Contributor

I didn't check the javadoc included, but be sure a note for cyclical
objects is there.
On Feb 27, 2016 11:11 AM, "John Aylward" john@johnanddalene.net wrote:

I think the toMap and toList are reasonable names as they imply a
conversion, which is what the implementation is doing.
On Feb 27, 2016 11:05 AM, "Sean Leary" notifications@github.com wrote:

Seems like a reasonable enhancement.

Should the methods be renamed as getters, e.g. getMap() instead of
toMap() ?
Should the return values be Map and List<?> ?

Otherwise, no objections here.
Any other opinions?


Reply to this email directly or view it on GitHub
#203 (comment).

@joeferner
Copy link
Contributor Author

@johnjaylward Just added the warning to both methods

@stleary
Copy link
Owner

stleary commented Feb 28, 2016

What problem does this code solve?
Provide methods to facilitate access to a shallow copy of the data stored in JSONObject and JSONArray. During the copy, nested JSONObjects are converted into maps and nested JSONArrays are converted into lists.

Changes to the API?
Yes, 2 new methods are added:

JSONArray: public List<Object> toList();
JSONObject public Map<String, Object> toMap();

Changes to how the code behaves?
No.

Does it break the unit tests?
No, but new unit tests will be added after this commit. See stleary/JSON-Java-unit-test#43.

Will this require a new release?
No, this will be rolled into the next release, which is not yet scheduled.

Should the documentation be updated?
The online JavaDoc should be split into latest release and latest code branches. The latest code branch should be updated with the new methods. The readme file will be updated to point to both JavaDoc pages. It is not necessary to complete this before merging the code.

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

Successfully merging this pull request may close these issues.

3 participants