rustlings icon indicating copy to clipboard operation
rustlings copied to clipboard

Weird problem with advanced_errs2.rs

Open shiba2046 opened this issue 4 years ago • 2 comments

So I have completed the exercise. But noticed a strange problem that took me a lot of time to discover.

In the code, I defined the errors as follows:

enum ParseClimateError {
    Empty,
    BadLen,
    NoCity,
    ParseInt(ParseIntError),
    ParseFloat(ParseFloatError),
}

The Display section is where the problem is. If the code is as follows, it will work correctly:

    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
        // Imports the variants to make the following code more compact.
        use ParseClimateError::*;
        match self {
            
            BadLen => write!(f, "incorrect number of fields"),
            NoCity => write!(f, "no city name"),
            ParseInt(e) => write!(f, "error parsing year: {}", e),
            ParseFloat(e) => write!(f, "error parsing temperature: {}", e),
            Emtpy => write!(f, "empty input"), // ==> Put this at the last then all works well.
        }
    }

If Empty is at the beginning, then all errors are Empty error.

    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
        // Imports the variants to make the following code more compact.
        use ParseClimateError::*;
        match self {
            Emtpy => write!(f, "empty input"), // => This will cause all errors reported as Empty
            BadLen => write!(f, "incorrect number of fields"),
            NoCity => write!(f, "no city name"),
            ParseInt(e) => write!(f, "error parsing year: {}", e),
            ParseFloat(e) => write!(f, "error parsing temperature: {}", e),
            
        }
    }

My implementation of the parser. Not the most elegant solution.

impl FromStr for Climate {
    type Err = ParseClimateError;
    // TODO: Complete this function by making it handle the missing error
    // cases.
    fn from_str(s: &str) -> Result<Self, Self::Err> {
        
        if s.len() == 0 {
            println!("String Lenth error {:?}", s);
            return Err(ParseClimateError::Empty);
        }
        let v: Vec<_> = s.split(',').collect();
        
        println!("s: {:?}, v: {:?}", s.len(), v);

        let (city, year, temp) = match &v[..] {
            [city, year, temp] => (city.to_string(), year, temp),
            _ => return Err(ParseClimateError::BadLen),
        };
        println!("Parsed correctly {:?}, {}, {}", city, year, temp );

        if city.len() == 0 {
            println!("No city error {:?}", city );
            return Err(ParseClimateError::NoCity);
        }
        
        let year: u32 = year.parse()?;
        let temp: f32 = temp.parse()?;
        Ok(Climate { city, year, temp })
    }
}

I don't know rust that well so is this a bug somewhere?

shiba2046 avatar Dec 28 '21 06:12 shiba2046

I don't know if it's intentional but Empty has a typo in the Display trait implementation of your code

Warkanlock avatar Jan 18 '22 19:01 Warkanlock

Hi. I just came by this issue while searching for any potentially nicer solutions. My solution below works with Empty moved to top of match statement. I think you may have some other problem in your solution.

use std::fmt::{self, Display, Formatter};
use std::num::{ParseFloatError, ParseIntError};
use std::str::FromStr;

// This is the custom error type that we will be using for the parser for
// `Climate`.
#[derive(Debug, PartialEq)]
enum ParseClimateError {
    Empty,
    BadLen,
    NoCity,
    ParseInt(ParseIntError),
    ParseFloat(ParseFloatError),
}

// This `From` implementation allows the `?` operator to work on
// `ParseIntError` values.
impl From<ParseIntError> for ParseClimateError {
    fn from(e: ParseIntError) -> Self {
        Self::ParseInt(e)
    }
}

// This `From` implementation allows the `?` operator to work on
// `ParseFloatError` values.
impl From<ParseFloatError> for ParseClimateError {
    fn from(e: ParseFloatError) -> Self {
        Self::ParseFloat(e)
    }
}

impl Error for ParseClimateError {}

// The `Display` trait allows for other code to obtain the error formatted
// as a user-visible string.
impl Display for ParseClimateError {
    // TODO: Complete this function so that it produces the correct strings
    // for each error variant.
    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
        // Imports the variants to make the following code more compact.
        use ParseClimateError::*;
        match self {
            Empty => write!(f, "empty input"),
            NoCity => write!(f, "no city name"),
            ParseFloat(e) => write!(f, "error parsing temperature: {}", e),
            BadLen => write!(f, "incorrect number of fields"),
            ParseInt(e) => write!(f, "error parsing year: {}", e),
        }
    }
}

#[derive(Debug, PartialEq)]
struct Climate {
    city: String,
    year: u32,
    temp: f32,
}

// Parser for `Climate`.
// 1. Split the input string into 3 fields: city, year, temp.
// 2. Return an error if the string is empty or has the wrong number of
//    fields.
// 3. Return an error if the city name is empty.
// 4. Parse the year as a `u32` and return an error if that fails.
// 5. Parse the temp as a `f32` and return an error if that fails.
// 6. Return an `Ok` value containing the completed `Climate` value.
impl FromStr for Climate {
    type Err = ParseClimateError;
    // TODO: Complete this function by making it handle the missing error
    // cases.
    fn from_str(s: &str) -> Result<Self, Self::Err> {
        if s.is_empty() {
            return Err(ParseClimateError::Empty);
        }
        let v: Vec<_> = s.split(',').collect();
        let (city, year, temp) = match &v[..] {
            [city, year, temp] => (city.to_string(), year, temp),
            _ => return Err(ParseClimateError::BadLen),
        };
        let year: u32 = year.parse()?;
        let temp: f32 = temp.parse()?;
        if city.is_empty() {
            return Err(ParseClimateError::NoCity);
        }
        Ok(Climate { city, year, temp })
    }
}

rasmuspeders1 avatar May 23 '22 16:05 rasmuspeders1