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

The way to serialize enum as number and duration #163

Closed
ramsayleung opened this issue Dec 10, 2020 · 6 comments
Closed

The way to serialize enum as number and duration #163

ramsayleung opened this issue Dec 10, 2020 · 6 comments
Labels
enhancement New feature or request

Comments

@ramsayleung
Copy link
Owner

ramsayleung commented Dec 10, 2020

Is your feature request related to a problem? Please describe.
As #127 pointing out that AudioAnalysisSection::mode and AudioFeatures::mode are f32s but should be Option<Mode>s where enum Mode { Major, Minor } as it is more useful, it needs to serialize enum to number and deserialize number to enum.

Describe the solution you'd like

The serde official documentation provides a recommended way to serialize enum to number, but this solution needs an external crate named serde_repr, and I think we should be cautious to introduce a new dependency since it will increase the compile time.

Describe alternatives you've considered
there is another solution to convert enum value to an integer, but needs unsafe operation, check this link for more details

Additional context
I am trying to figure out that is there a more lightweight way to do so, if you have any suggestion, feel free to help :)

PS:

I am still looking for a way to serialize/deserialize Duration, and I find some tracking issues:

@ramsayleung ramsayleung added the enhancement New feature or request label Dec 10, 2020
@ramsayleung ramsayleung changed the title The way to serialize enum as number The way to serialize enum as number and duration Dec 11, 2020
@ramsayleung
Copy link
Owner Author

I figured out that the better choice is that we de/serialize std::time::Duration manually with our own de/serialize functions, just deserializing Duration from millisecond and serializing Duration to millisecond:

use serde::{de, Deserialize, Serialize, Serializer};
use std::{fmt, time::Duration};
struct DurationVisitor;
impl<'de> de::Visitor<'de> for DurationVisitor {
    type Value = Duration;
    fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
        write!(formatter, "a milliseconds represents std::time::Duration")
    }
    fn visit_u64<E>(self, v: u64) -> Result<Self::Value, E>
    where
        E: de::Error,
    {
        Ok(Duration::from_millis(v))
    }
}
fn from_duration_ms<'de, D>(d: D) -> Result<Duration, D::Error>
where
    D: de::Deserializer<'de>,
{
    d.deserialize_u64(DurationVisitor)
}

fn to_duration_ms<S>(x: &Duration, s: S) -> Result<S::Ok, S::Error>
where
    S: Serializer,
{
    s.serialize_u64(x.as_millis() as u64)
}
#[derive(Deserialize, Serialize, Debug)]
struct Track {
    #[serde(
        deserialize_with = "from_duration_ms",
        serialize_with = "to_duration_ms",
        rename = "duration_ms"
    )]
    duration: Duration,
}
fn main() {
    let json = r#"
    {
        "duration_ms": 31231232
    }
    "#;
    let track: Track = serde_json::from_str(json).unwrap();
    println!("{:?}", serde_json::to_string(&track).unwrap());
}

@marioortizmanero
Copy link
Collaborator

I think manually deserializing std::time::Duration is a great choice as well, since it's not that much code. As to the AudioAnalysisSection and AudioAnalysisSegment issue, I don't fully understand it. What exactly is enum Mode { Major, Minor } and where in the models does it come from?

@ramsayleung
Copy link
Owner Author

ramsayleung commented Dec 14, 2020

The mode filed from AudioAnalysisSection is an integer, which indicates the modality (major or minor) of a track, the type of scale from which its melodic content is derived. This field will contain a 0 for minor, a 1 for major, or a -1 for no result.

the mode field was stored as a f32 before, and Koxiaet suggests that it's would be better to replace f32 with enum Mode { Major, Minor}. Thus it's necessary to figure out how to deserialize a 0/1/-1 to enum Mode. Perhaps we could manually deserializing f32 to enum Mode as what we do for std::time::Duration.

@marioortizmanero
Copy link
Collaborator

marioortizmanero commented Dec 14, 2020

Ah I was confused because rather than f32 it could've just used i32 or i8 to represent 0, 1, or -1. I would do exactly as std::time::Duration, yeah. It seems like the easiest way to go. If we end up needing more of this in the future then we can pull serde_repr.

@ramsayleung
Copy link
Owner Author

Ah I was confused because rather than f32 it could've just used i32 or i8 to represent 0, 1, or -1.

Yep, I have to confess it's a design mistake, i8 is a better choice to represent 0, 1, or -1.

If we end up needing more of this in the future then we can pull serde_repr.

Agree! we should avoid to introduce any unnecessary dependency as well.

@ramsayleung
Copy link
Owner Author

My solution to manually deserialize 0/1/-1 to enum Mode:

#[derive(Debug, PartialEq)]
enum Mode {
    Minor,
    Major,
    NoResult,
}
struct ModeVisitor;
impl<'de> de::Visitor<'de> for ModeVisitor {
    type Value = Mode;
    fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
        write!(formatter, "valid number: 0, 1, -1")
    }

    fn visit_u64<E>(self, v: u64) -> Result<Self::Value, E>
    where
        E: de::Error,
    {
        match v {
            0 => Ok(Mode::Minor),
            1 => Ok(Mode::Major),
            value @ _ => {
                println!("invalid value: {:?}", value);
                Err(de::Error::invalid_value(de::Unexpected::Unsigned(v), &self))
            }
        }
    }

    fn visit_i64<E>(self, v: i64) -> Result<Self::Value, E>
    where
        E: de::Error,
    {
        match v {
            0 => Ok(Mode::Minor),
            1 => Ok(Mode::Major),
            -1 => Ok(Mode::NoResult),
            value @ _ => {
                println!("invalid value: {:?}", value);
                Err(de::Error::invalid_value(de::Unexpected::Signed(v), &self))
            }
        }
    }
}

fn from_integer<'de, D>(d: D) -> Result<Mode, D::Error>
where
    D: de::Deserializer<'de>,
{
    d.deserialize_any(ModeVisitor)
}

fn to_integer<S>(x: &Mode, s: S) -> Result<S::Ok, S::Error>
where
    S: Serializer,
{
    match x {
        Mode::Minor => s.serialize_i8(0),
        Mode::Major => s.serialize_i8(1),
        Mode::NoResult => s.serialize_i8(-1),
    }
}

#[derive(Deserialize, Serialize, Debug)]
struct Track {
    #[serde(deserialize_with = "from_integer", serialize_with = "to_integer")]
    mode: Mode,
}

fn main() {
    let json = r#"
    {
        "mode": 1
    }
    "#;
    let track: Track = serde_json::from_str(json).unwrap();
    assert_eq!(track.mode, Mode::Major);
    println!("{:?}", serde_json::to_string(&track).unwrap());
}

You may be curious about the visit_i64 and visit_u64 method, since if I set mode filed to -1, Serde will call visit_i64, then I set mode field to 1, it will call the visit_u64, I am not sure how does Serde dispatch to related visit method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants