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

Why not impl TryInto<InnerType> for Value? #902

Open
stackinspector opened this issue Jul 2, 2022 · 4 comments · May be fixed by #947
Open

Why not impl TryInto<InnerType> for Value? #902

stackinspector opened this issue Jul 2, 2022 · 4 comments · May be fixed by #947

Comments

@stackinspector
Copy link

The InnerType can be bool, u64, i64, f64, String, Vec<Value> or other types implemented as_{type}, to build a more convient API.

@cherryblossom000
Copy link

cherryblossom000 commented Jul 4, 2022

I agree. Code like

let map = match value
    .method()
    .another_method()
{
    serde_json::Value::Object(o) => o,
    _ => panic!("expected an object")
};

could be written a lot cleaner like

let map: serde_json::Map<_, _> = value
    .method()
    .another_method()
    .try_into()
    .expect("expected an object");

By the way, it would be impl TryFrom<Value> for InnerType. Like Into, there’s a blanket impl for TryInto so it shouldn’t be directly implemented.

@szymek156
Copy link

Funny how I started implementing it on my own. I would add to that, do impl on the borrow of Value impl TryInto<&Value> for XXX in order to not consume it:

use core::convert::TryInto;

impl TryFrom<&Value> for i64 {
    type Error = Error;

    fn try_from(value: &Value) -> Result<Self, Self::Error> {
        value
            .as_i64()
            .ok_or(serde::de::Error::custom("Cannot interpret Value as i64"))
    }
}

impl<'a> TryFrom<&'a Value> for &'a str {
    type Error = Error;

    fn try_from(value: &'a Value) -> Result<Self, Self::Error> {
        value
            .as_str()
            .ok_or(serde::de::Error::custom("Cannot interpret Value as &str"))
    }
}

that would allow you to do:

    fn get_property<'a, T>(val: &'a Value, name: &str) -> std::result::Result<T, T::Error>
    where
        T: TryFrom<&'a Value>,
        <T as std::convert::TryFrom<&'a Value>>::Error: serde::de::Error,
    {
        val.as_object()
            .ok_or(serde::de::Error::custom("value is not an object"))?
            .get(name)
            .ok_or(serde::de::Error::custom("no property found"))?
            .try_into()
    }


    #[test]
    fn it_works() {
        let j_son = json!({"number": 1, "str": "hello"});

        // Don't consume j_son, so I can call it many times
        assert_eq!(get_property::<i64>(&j_son, "number").unwrap(), 1);
        assert_eq!(get_property::<&str>(&j_son, "str").unwrap(), "hello");
    }

@cherryblossom000
Copy link

There should be an impl for both TryFrom<Value> and TryFrom<&Value>. My use case was that I wanted to consume the Value.

@phayes
Copy link

phayes commented Nov 2, 2022

I also would like to see this. I have an use-case where I want to delay fully deserializing a Value into a concrete type until much deeper into the code. So having TryInto for Value would be useful for me. I'll make a PR.

@phayes phayes linked a pull request Nov 2, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants