Json cleanup and improvements #15238
Conversation
Why?
It's not feasible to use |
|
Calling Now I consider it again, I think it is good to keep |
Its main purpose would be generic code, allowing
That would be the desirable approach. Here's an example you can build from, if you do wish to try. |
|
@huonw I have addressed your comments. |
| /// Shortcut function to decode a `&str` in JSON format into an object | ||
| #[inline] | ||
| pub fn decode<T: ::Decodable<Decoder, DecoderError>>(s: &str) -> DecodeResult<T> { | ||
| let json = from_str(s).unwrap(); |
There was a problem hiding this comment.
This seems like a legitimate possibility for a decode error, why unwrap here?
There was a problem hiding this comment.
You are right.... I should use try! here instead of unwrap
|
It looks like the Travis build timed out |
* Tried to make the code more idiomatic * Renamed the `wr` field of the `Encoder` and `PrettyEncoder` structs to `writer` * Replaced some `from_utf8` by `from_utf8_owned` to avoid unnecessary allocations * Removed unnecessary `unsafe` code
Now you can just use `json::encode` and `json::decode`, which is very practical **Deprecated `Encoder::str_encode` in favor of `json::encode`** [breaking-change]
Fixed some errors, removed some code examples and added usage of the `encode` and `decode` functions.
|
It looks like the code snippets in the documentation had unused imports. I have removed them, so everything should be fine now. |
### Breaking changes: * **Removed unnecessary `box` from enum variant (`Object(Box<Object>)` becomes `Object(Object)`)** * **Deprecated `Encoder::str_encode`** ### Other changes: * Tried to make the code more idiomatic * Renamed the `wr` field of the `Encoder` and `PrettyEncoder` structs to `writer` * Replaced some `from_utf8` by `from_utf8_owned` to avoid unnecessary allocations * Removed unnecessary `unsafe` code * Added `encode` and `decode` shortcut functions * Implemented `FromStr` for `Json` * Implemented `ToJson` for tuples of arity up to 12 * Fixed some details in the documentation ### Questions * ~~The `encode` shortcut function does the same as the `Encoder::str_encode` function. It seems wrong to me that two functions do exactly the same. Should we deprecate `Encoder::str_encode`?~~ * ~~Do we really want the ToJson trait for tuples? At the moment we have it for (), (A, B), (A, B, C). I would like to remove them.~~ * ~~We are using `String` as key in the `TreeMap` representing a `Json` object. It would be better to use `&str`, but this would require to annotate lots of lifetimes. Is there any easy solution for this?~~ * There is a lot of duplicated code (`PrettyEncoder` copies about 50 lines from `Encoder`). In an OO language this could be solved very elegantly by using inheritance and overriding. What can we do here to reduce the amount of boilerplate? [breaking-change]
Fix missing terminator in pattern matching of consts fix rust-lang#15238
Breaking changes:
boxfrom enum variant (Object(Box<Object>)becomesObject(Object))Encoder::str_encodeOther changes:
wrfield of theEncoderandPrettyEncoderstructs towriterfrom_utf8byfrom_utf8_ownedto avoid unnecessary allocationsunsafecodeencodeanddecodeshortcut functionsFromStrforJsonToJsonfor tuples of arity up to 12Questions
Theencodeshortcut function does the same as theEncoder::str_encodefunction. It seems wrong to me that two functions do exactly the same. Should we deprecateEncoder::str_encode?Do we really want the ToJson trait for tuples? At the moment we have it for (), (A, B), (A, B, C). I would like to remove them.We are usingStringas key in theTreeMaprepresenting aJsonobject. It would be better to use&str, but this would require to annotate lots of lifetimes. Is there any easy solution for this?PrettyEncodercopies about 50 lines fromEncoder). In an OO language this could be solved very elegantly by using inheritance and overriding. What can we do here to reduce the amount of boilerplate?[breaking-change]