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 JSON.parse return JSON::Any, and make it easy to traverse a dynamic JSON structure. Fixes #1832 #1883

Merged
merged 1 commit into from
Nov 12, 2015

Conversation

asterite
Copy link
Member

This is the implementation for #1832

I decided to name the "cast" methods as_i, as_s, etc., to avoid confusion: if you do to_s on a value that isn't necessarily a String, will the value be converted to a String? Or if you do to_i and the underlying value is a String that can be parsed as an int, will it work? Because as_... is not related to to_... methods, which usually perform conversion/parsing, the distinction is a bit more clear.

end

it "gets array" do
JSON.parse(%([1, 2, 3])).as_a.should eq([1, 2, 3])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What will be the type in this case? Array(JSON::Any) ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or is it Array(JSON::Type) ?

Source says just @raw as Array, so I am not exactly sure what it will do. Probably latter, isn't it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Array(JSON::Type). I think as_a maybe won't be useful, right? You'd traverse it with #[]... Or we can make as_a to map the internal values to JSON::Any but that's a separate array -> more memory allocated

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see.

For sure not in a scope of this PR: Could it be possible to make a thin wrapper JSON::AnyArray, that will wrap Array(JSON::Type) and will provide neat interface, like #[], #[]?, #each that will on the fly re-wrap values of type JSON::Type in JSON::Any ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same suggestion goes about #as_h.

If you like this idea, I can spike it after this PR gets merged.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@waterlink We could do it, but I don't think it's necessary. JSON::Any already lets you traverse the structure as an array or hash, so I don't know what JSON::AnyArray would add. We can wait until we have a use case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, and I added return types so it's clear what is being returned :-)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair :)

@waterlink
Copy link
Contributor

👍

@sdogruyol
Copy link
Member

Awesome work 👍

asterite pushed a commit that referenced this pull request Nov 12, 2015
Make `JSON.parse` return `JSON::Any`, and make it easy to traverse a dynamic JSON structure. Fixes #1832
@asterite asterite merged commit 1807968 into master Nov 12, 2015
@asterite asterite deleted the json_any branch November 12, 2015 13:32
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.

None yet

4 participants