Json ans unwrap, why I need to write so much Rust code?

help
1

Hi,

I am learning Rust for around a year. I like it. But the whole unwrap and full case handling makes the code look so not elegant. I am writing the same code in Python, C++, Rust for learning Rust.

Can someone review this simple Point struct, specially the json serialization part and give good advice how to make it simpler and cleaner?

Full repo for the point struct in the other languages: GitHub - petrasvestartas/session

use std::fmt;
use serde::{Deserialize, Serialize};
use uuid::Uuid;
use crate::Color;
use serde_json::Value;
use std::fs;

/// A point with XYZ coordinates and display properties.
///
/// # Fields
/// * `x` - X coordinate
/// * `y` - Y coordinate  
/// * `z` - Z coordinate
/// * `guid` - Unique identifier
/// * `name` - Point name
/// * `pointcolor` - Point color
/// * `width` - Point width
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct Point {
    pub x: f32,
    pub y: f32,
    pub z: f32,
    pub guid: Uuid,
    pub name: String,
    pub pointcolor: Color,
    pub width: f32,
}

impl Point {
    /// Create new point.
    pub fn new(x: f32, y: f32, z: f32) -> Self {
        Self {
            x,
            y,
            z,
            guid: Uuid::new_v4(),
            name: "Point".to_string(),
            pointcolor: Color::white(),
            width: 1.0,
        }
    }
}

// JSON
impl Point {
    /// Convert to JSON data.
    pub fn to_json_data(&self, minimal: bool) -> Value {
        if minimal {
            serde_json::json!({"dtype": "Point", "x": self.x, "y": self.y, "z": self.z})
        } else {
            serde_json::json!({
                "dtype": "Point", "x": self.x, "y": self.y, "z": self.z,
                "guid": self.guid.to_string(), "name": self.name,
                "pointcolor": self.pointcolor.to_json_data(false), "width": self.width
            })
        }
    }

    /// Create from JSON data.
    pub fn from_json_data(data: &Value) -> Self {
        Point {
            x: data["x"].as_f32().unwrap(),
            y: data["y"].as_f32().unwrap(),
            z: data["z"].as_f32().unwrap(),
            guid: data.get("guid").and_then(|g| g.as_str()).and_then(|s| Uuid::parse_str(s).ok()).unwrap_or_else(Uuid::new_v4),
            name: data.get("name").and_then(|n| n.as_str()).unwrap_or("Point").to_string(),
            pointcolor: data.get("pointcolor").map(Color::from_json_data).unwrap_or_else(Color::white),
            width: data.get("width").and_then(|w| w.as_f32()).unwrap_or(1.0),
        }
    }

    /// Serialize to JSON string.
    pub fn to_json(&self, minimal: bool) -> String {
        serde_json::to_string(&self.to_json_data(minimal)).unwrap()
    }

    /// Deserialize from JSON string.
    pub fn from_json(json_str: &str) -> Self {
        let data: Value = serde_json::from_str(json_str).unwrap();
        Self::from_json_data(&data)
    }
}

impl fmt::Display for Point {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        write!(
            f, 
            "Point({}, {}, {}, {}, {}, {}, {})", 
            self.x, self.y, self.z, self.guid, self.name, self.pointcolor, self.width
        )
    }
}
2

You should generally be using #[derive(serde::Serialize, serde::Deserialize)] instead. This will generate code to propagate errors properly and can handle inserting default values when desired. It will also avoid the cost of constructing a temporary serde_json::Value.

Considering only keeping the functions you have and improving the error handling, you should be propagating the errors instead of unwrap()ing them, so that your program can report invalid data without panicking:

    pub fn from_json_data(data: &Value) -> Result<Self, &'static str> {
        Ok(Point {
            x: data.get("x").ok_or("missing x")?.as_f32().ok_or("invalid x")?,
            y: data.get("y").ok_or("missing y")?.as_f32().ok_or("invalid y")?,
            z: data.get("z").ok_or("missing z")?.as_f32().ok_or("invalid z")?,
            ...
        })
    }
}

Returning a string isn't the best error reporting style, but it's sufficient to demonstrate the point of using ?. And you don't have to repeat the code 3 times if you write a helper function — but you don't need to do any of that if you derive(serde::Deserialize), which handles propagating each possibility of missing or invalid data.

3 Likes
3

I am trying to have similar methods as in C++ and Python from and to json.

But what you are saying does it mean, that I do not need these methods at all? Could you provide an example what will change when I add these serde derive decorations?

The reason to ask is that sometimes you want to serialize all the attributes of a struct, and sometimes you want specifically select a few ones.

4

That's unidiomatic use of serde (and hence unidiomatic use of Rust), and is less efficient, because constructing a serde_json::Value requires a large number of of small allocations and building hash tables. None of that is typically required when the serialization/deserialization is done directly.

I don't think of this as a common requirement, but if it’s needed, you can do this by defining other structs to convert your Point into before serialization. Those structs can even borrow the nontrivial fields of Point (like name) to avoid needing to clone the strings.

6 Likes
5

You can find examples in the crate's documentation. For example, here and here (as you can see, it's quite flexible). The API is also documented, like any crate; you can find that here.

The derive attribute adds automatically generated code for the compiler so you don't have to write it yourself. You can actually see the generated code with some tools like cargo-expand and rust-analyzer or with IDEs like RustRover.

Some of the derives are part of the standard library, like #[derive(Clone)], that you've used in your example, and some come with additional crates where they're implemented as a procedural macros to make their user's life easier.

Using the derive attribute doesn't mean you won't need any method to handle the potential errors, but you won't need to handle the details for each field.

6

I wrote some here on my project, aiming for them to be as simple as possible. This is for YAML, but any JSON is also a valid YAML so that this code would read any JSON document as well.

7

Thanks, but I was only answering the OP. Unfortunately, they don't seem to monitor this issue any more.

8

Rust doesn't use exceptions, it's why unwrap. You generally can avoid unwrap using '?'. But, as it was recommended here, better to use unwrap_or. I wrote about that in my book.