Added convenience methods to the Json class #12780
Merged
+247
−0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Travis says you have styling errors (you can check that locally by running |
|
Fixed! Thanks, I didn't know about |
|
@alexcrichton Good idea. I'll do the same thing with List while I'm at it. |
|
@alexcrichton I've addressed your notes from yesterday. |
|
|
||
| /// If the Json value is a List, returns the associated vector. | ||
| /// Returns None otherwise. | ||
| //pub fn as_list<'a>(&'a self) -> Option<&'a ~[Json]> { |
There was a problem hiding this comment.
This line should get deleted
|
This looks good to me, thanks! Could you also squash the commits into one? |
|
@alexcrichton Done! |
Fixed some styling issues with trailing whitespace. - Removed redundant functions. - Renamed `get` to `find` - Renamed `get_path` to `find_path` - Renamed `find` to `search` - Changed as_object and as_list to return Object and List rather than the underlying implementation types of TreeMap<~str,Json> and ~[Json] - Refactored find_path to use a fold() instead of recursion Formatting fixes. Fixed spacing, deleted comment. Added convenience methods and accompanying tests to the Json class. Updated tests to expect less pointer indirection.
|
/facepalm I forgot to update the tests to expect less pointer indirection after changing the method signatures. Fixed, make-check'ed, make-tidy'ed, committed, squashed, rebased, pushed. I'll get the hang of this yet! |
bors
added a commit
that referenced
this pull request
Mar 11, 2014
This is my first non-docs contribution to Rust, so please let me know what I can fix. I probably should've submitted this to the mailing list first for comments, but it didn't take too long to implement so I figured I'd just give it a shot. These changes are modeled loosely on the [JsonNode API](http://jackson.codehaus.org/1.7.9/javadoc/org/codehaus/jackson/JsonNode.html) provided by the [Jackson JSON processor](http://jackson.codehaus.org/). Many common use cases for parsing JSON involve pulling one or more fields out of an object, however deeply nested. At present, this requires writing a pyramid of match statements. The added methods in this PR aim to make this a more painless process. **Edited to reflect final implementation** Example JSON: ```json { "successful" : true, "status" : 200, "error" : null, "content" : { "vehicles" : [ {"make" : "Toyota", "model" : "Camry", "year" : 1997}, {"make" : "Honda", "model" : "Accord", "year" : 2003} ] } } ``` Accessing "successful": ```rust let example_json : Json = from_str("...above json...").unwrap(); let was_successful: Option<bool> = example_json.find(&~"successful").and_then(|j| j.as_boolean()); ``` Accessing "status": ```rust let example_json : Json = from_str("...above json...").unwrap(); let status_code : Option<f64> = example_json.find(&~"status").and_then(|j| j.as_number()); ``` Accessing "vehicles": ```rust let example_json : Json = from_str("...above json...").unwrap(); let vehicle_list: Option<List> = example_json.search(&~"vehicles").and_then(|j| j.as_list()); ``` Accessing "vehicles" with an explicit path: ```rust let example_json : Json = from_str("...above json...").unwrap(); let vehicle_list: Option<List> = example_json.find_path(&[&~"content", &~"vehicles"]).and_then(|j| j.as_list()); ``` Accessing "error", which might be null or a string: ```rust let example_json : Json = from_str("...above json...").unwrap(); let error: Option<Json> = example_json.find(&~"error"); if error.is_null() { // This would be nicer as a match, I'm just illustrating the boolean test methods println!("Error is null, everything's fine."); } else if error.is_str(){ println!("Something went wrong: {}", error.as_string().unwrap()); } ``` Some notes: * Macros would help to eliminate some of the repetitiveness of the implementation, but I couldn't use them due to #4621. (**Edit**: There is no longer repetitive impl. Methods were simplified to make them more composable.) * Would it be better to name methods after the Json enum type (e.g. `get_string`) or the associated Rust built-in? (e.g. `get_str`) * TreeMap requires its keys to be &~str. Because of this, all of the new methods required &~str for their parameters. I'm uncertain what the best approach to fixing this is: neither demanding an owned pointer nor allocating within the methods to appease TreeMap's find() seems desirable. If I were able to take &str, people could put together paths easily with `"foo.bar.baz".split('.').collect();` (**Edit**: Follow on investigation into making TreeMap able to search by Equiv would be worthwhile.) * At the moment, the `find_<sometype>` methods all find the first match for the provided key and attempt to return that value if it's of the specified type. This makes sense to me, but it's possible that users would interpret a call to `find_boolean("successful")` as looking for the first "successful" item that was a boolean rather than looking for the first "successful" and returning None if it isn't boolean. (**Edit**: No longer relevant.) I hope this is helpful. Any feedback is appreciated!
|
is_* methods could be implemented in terms of as_* methods (less duplication): |
|
That's a good idea. I've thought of other a couple of cleanup ideas for these methods since submitting. I've opened #12829 to track this. |
flip1995
pushed a commit
to flip1995/rust
that referenced
this pull request
May 17, 2024
…ns, r=Manishearth Ignore `_to_string` lints in code `from_expansion` Includes the `string_to_string` and `str_to_string` lints. changelog: [`str_to_string`]: Ignore code from expansion changelog: [`string_to_string`]: Ignore code from expansion
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is my first non-docs contribution to Rust, so please let me know what I can fix. I probably should've submitted this to the mailing list first for comments, but it didn't take too long to implement so I figured I'd just give it a shot.
These changes are modeled loosely on the JsonNode API provided by the Jackson JSON processor.
Many common use cases for parsing JSON involve pulling one or more fields out of an object, however deeply nested. At present, this requires writing a pyramid of match statements. The added methods in this PR aim to make this a more painless process.
Edited to reflect final implementation
Example JSON:
{ "successful" : true, "status" : 200, "error" : null, "content" : { "vehicles" : [ {"make" : "Toyota", "model" : "Camry", "year" : 1997}, {"make" : "Honda", "model" : "Accord", "year" : 2003} ] } }Accessing "successful":
Accessing "status":
Accessing "vehicles":
Accessing "vehicles" with an explicit path:
Accessing "error", which might be null or a string:
Some notes:
get_string) or the associated Rust built-in? (e.g.get_str)"foo.bar.baz".split('.').collect();(Edit: Follow on investigation into making TreeMap able to search by Equiv would be worthwhile.)find_<sometype>methods all find the first match for the provided key and attempt to return that value if it's of the specified type. This makes sense to me, but it's possible that users would interpret a call tofind_boolean("successful")as looking for the first "successful" item that was a boolean rather than looking for the first "successful" and returning None if it isn't boolean. (Edit: No longer relevant.)I hope this is helpful. Any feedback is appreciated!