From 2dc93caddad43821743e4903d89b355df58d7a49 Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Thu, 24 Jun 2021 21:33:41 -0500 Subject: [PATCH] fix(from_str, try_from_into): custom error types Remove the use of trait objects as errors from `from_str` and `try_from_into`; they seem to have caused a lot of confusion in practice. (Also, it's considered best practice to use custom error types instead of boxed errors in library code.) Instead, use custom error enums, and update hints accordingly. Hints also provide some guidance about converting errors, which could be covered more completely in a future advanced errors section. Also move from_str to directly after the similar exercise `from_into`, for the sake of familiarity when solving. --- exercises/conversions/from_str.rs | 56 ++++++++++++++----- exercises/conversions/try_from_into.rs | 74 ++++++++++++++++++-------- info.toml | 53 ++++++++++-------- 3 files changed, 127 insertions(+), 56 deletions(-) diff --git a/exercises/conversions/from_str.rs b/exercises/conversions/from_str.rs index 4beebacd..6e9e699d 100644 --- a/exercises/conversions/from_str.rs +++ b/exercises/conversions/from_str.rs @@ -1,16 +1,31 @@ -// This does practically the same thing that TryFrom<&str> does. +// from_str.rs +// This is similar to from_into.rs, but this time we'll implement `FromStr` +// and return errors instead of falling back to a default value. // Additionally, upon implementing FromStr, you can use the `parse` method // on strings to generate an object of the implementor type. // You can read more about it at https://doc.rust-lang.org/std/str/trait.FromStr.html -use std::error; +use std::num::ParseIntError; use std::str::FromStr; -#[derive(Debug)] +#[derive(Debug, PartialEq)] struct Person { name: String, age: usize, } +// We will use this error type for the `FromStr` implementation. +#[derive(Debug, PartialEq)] +enum ParsePersonError { + // Empty input string + Empty, + // Incorrect number of fields + BadLen, + // Empty name field + NoName, + // Wrapped error from parse::() + ParseInt(ParseIntError), +} + // I AM NOT DONE // Steps: @@ -24,7 +39,7 @@ struct Person { // If everything goes well, then return a Result of a Person object impl FromStr for Person { - type Err = Box; + type Err = ParsePersonError; fn from_str(s: &str) -> Result { } } @@ -40,7 +55,7 @@ mod tests { #[test] fn empty_input() { - assert!("".parse::().is_err()); + assert_eq!("".parse::(), Err(ParsePersonError::Empty)); } #[test] fn good_input() { @@ -52,41 +67,56 @@ mod tests { } #[test] fn missing_age() { - assert!("John,".parse::().is_err()); + assert!(matches!( + "John,".parse::(), + Err(ParsePersonError::ParseInt(_)) + )); } #[test] fn invalid_age() { - assert!("John,twenty".parse::().is_err()); + assert!(matches!( + "John,twenty".parse::(), + Err(ParsePersonError::ParseInt(_)) + )); } #[test] fn missing_comma_and_age() { - assert!("John".parse::().is_err()); + assert_eq!("John".parse::(), Err(ParsePersonError::BadLen)); } #[test] fn missing_name() { - assert!(",1".parse::().is_err()); + assert_eq!(",1".parse::(), Err(ParsePersonError::NoName)); } #[test] fn missing_name_and_age() { - assert!(",".parse::().is_err()); + assert!(matches!( + ",".parse::(), + Err(ParsePersonError::NoName | ParsePersonError::ParseInt(_)) + )); } #[test] fn missing_name_and_invalid_age() { - assert!(",one".parse::().is_err()); + assert!(matches!( + ",one".parse::(), + Err(ParsePersonError::NoName | ParsePersonError::ParseInt(_)) + )); } #[test] fn trailing_comma() { - assert!("John,32,".parse::().is_err()); + assert_eq!("John,32,".parse::(), Err(ParsePersonError::BadLen)); } #[test] fn trailing_comma_and_some_string() { - assert!("John,32,man".parse::().is_err()); + assert_eq!( + "John,32,man".parse::(), + Err(ParsePersonError::BadLen) + ); } } diff --git a/exercises/conversions/try_from_into.rs b/exercises/conversions/try_from_into.rs index c0b5d986..b8ec4455 100644 --- a/exercises/conversions/try_from_into.rs +++ b/exercises/conversions/try_from_into.rs @@ -1,9 +1,9 @@ +// try_from_into.rs // TryFrom is a simple and safe type conversion that may fail in a controlled way under some circumstances. // Basically, this is the same as From. The main difference is that this should return a Result type // instead of the target type itself. // You can read more about it at https://doc.rust-lang.org/std/convert/trait.TryFrom.html use std::convert::{TryFrom, TryInto}; -use std::error; #[derive(Debug, PartialEq)] struct Color { @@ -12,12 +12,21 @@ struct Color { blue: u8, } +// We will use this error type for these `TryFrom` conversions. +#[derive(Debug, PartialEq)] +enum IntoColorError { + // Incorrect length of slice + BadLen, + // Integer conversion error + IntConversion, +} + // I AM NOT DONE // Your task is to complete this implementation // and return an Ok result of inner type Color. // You need to create an implementation for a tuple of three integers, -// an array of three integers and a slice of integers. +// an array of three integers, and a slice of integers. // // Note that the implementation for tuple and array will be checked at compile time, // but the slice implementation needs to check the slice length! @@ -25,20 +34,23 @@ struct Color { // Tuple implementation impl TryFrom<(i16, i16, i16)> for Color { - type Error = Box; - fn try_from(tuple: (i16, i16, i16)) -> Result {} + type Error = IntoColorError; + fn try_from(tuple: (i16, i16, i16)) -> Result { + } } // Array implementation impl TryFrom<[i16; 3]> for Color { - type Error = Box; - fn try_from(arr: [i16; 3]) -> Result {} + type Error = IntoColorError; + fn try_from(arr: [i16; 3]) -> Result { + } } // Slice implementation impl TryFrom<&[i16]> for Color { - type Error = Box; - fn try_from(slice: &[i16]) -> Result {} + type Error = IntoColorError; + fn try_from(slice: &[i16]) -> Result { + } } fn main() { @@ -46,15 +58,15 @@ fn main() { let c1 = Color::try_from((183, 65, 14)); println!("{:?}", c1); - // Since From is implemented for Color, we should be able to use Into + // Since TryFrom is implemented for Color, we should be able to use TryInto let c2: Result = [183, 65, 14].try_into(); println!("{:?}", c2); let v = vec![183, 65, 14]; - // With slice we should use `from` function + // With slice we should use `try_from` function let c3 = Color::try_from(&v[..]); println!("{:?}", c3); - // or take slice within round brackets and use Into + // or take slice within round brackets and use TryInto let c4: Result = (&v[..]).try_into(); println!("{:?}", c4); } @@ -65,15 +77,24 @@ mod tests { #[test] fn test_tuple_out_of_range_positive() { - assert!(Color::try_from((256, 1000, 10000)).is_err()); + assert_eq!( + Color::try_from((256, 1000, 10000)), + Err(IntoColorError::IntConversion) + ); } #[test] fn test_tuple_out_of_range_negative() { - assert!(Color::try_from((-1, -10, -256)).is_err()); + assert_eq!( + Color::try_from((-1, -10, -256)), + Err(IntoColorError::IntConversion) + ); } #[test] fn test_tuple_sum() { - assert!(Color::try_from((-1, 255, 255)).is_err()); + assert_eq!( + Color::try_from((-1, 255, 255)), + Err(IntoColorError::IntConversion) + ); } #[test] fn test_tuple_correct() { @@ -91,17 +112,17 @@ mod tests { #[test] fn test_array_out_of_range_positive() { let c: Result = [1000, 10000, 256].try_into(); - assert!(c.is_err()); + assert_eq!(c, Err(IntoColorError::IntConversion)); } #[test] fn test_array_out_of_range_negative() { let c: Result = [-10, -256, -1].try_into(); - assert!(c.is_err()); + assert_eq!(c, Err(IntoColorError::IntConversion)); } #[test] fn test_array_sum() { let c: Result = [-1, 255, 255].try_into(); - assert!(c.is_err()); + assert_eq!(c, Err(IntoColorError::IntConversion)); } #[test] fn test_array_correct() { @@ -119,17 +140,26 @@ mod tests { #[test] fn test_slice_out_of_range_positive() { let arr = [10000, 256, 1000]; - assert!(Color::try_from(&arr[..]).is_err()); + assert_eq!( + Color::try_from(&arr[..]), + Err(IntoColorError::IntConversion) + ); } #[test] fn test_slice_out_of_range_negative() { let arr = [-256, -1, -10]; - assert!(Color::try_from(&arr[..]).is_err()); + assert_eq!( + Color::try_from(&arr[..]), + Err(IntoColorError::IntConversion) + ); } #[test] fn test_slice_sum() { let arr = [-1, 255, 255]; - assert!(Color::try_from(&arr[..]).is_err()); + assert_eq!( + Color::try_from(&arr[..]), + Err(IntoColorError::IntConversion) + ); } #[test] fn test_slice_correct() { @@ -148,11 +178,11 @@ mod tests { #[test] fn test_slice_excess_length() { let v = vec![0, 0, 0, 0]; - assert!(Color::try_from(&v[..]).is_err()); + assert_eq!(Color::try_from(&v[..]), Err(IntoColorError::BadLen)); } #[test] fn test_slice_insufficient_length() { let v = vec![0, 0]; - assert!(Color::try_from(&v[..]).is_err()); + assert_eq!(Color::try_from(&v[..]), Err(IntoColorError::BadLen)); } } diff --git a/info.toml b/info.toml index ab4dc933..f5af8840 100644 --- a/info.toml +++ b/info.toml @@ -925,6 +925,27 @@ mode = "test" hint = """ Follow the steps provided right before the `From` implementation""" +[[exercises]] +name = "from_str" +path = "exercises/conversions/from_str.rs" +mode = "test" +hint = """ +The implementation of FromStr should return an Ok with a Person object, +or an Err with an error if the string is not valid. + +This is almost like the `from_into` exercise, but returning errors instead +of falling back to a default value. + +Hint: Look at the test cases to see which error variants to return. + +Another hint: You can use the `map_err` method of `Result` with a function +or a closure to wrap the error from `parse::`. + +Yet another hint: If you would like to propagate errors by using the `?` +operator in your solution, you might want to look at +https://doc.rust-lang.org/stable/rust-by-example/error/multiple_error_types/reenter_question_mark.html +""" + [[exercises]] name = "try_from_into" path = "exercises/conversions/try_from_into.rs" @@ -933,17 +954,19 @@ hint = """ Follow the steps provided right before the `TryFrom` implementation. You can also use the example at https://doc.rust-lang.org/std/convert/trait.TryFrom.html -You might want to look back at the exercise errors5 (or its hints) to remind -yourself about how `Box` works. +Hint: Is there an implementation of `TryFrom` in the standard library that +can both do the required integer conversion and check the range of the input? + +Another hint: Look at the test cases to see which error variants to return. -If you're trying to return a string as an error, note that neither `str` -nor `String` implements `error::Error`. However, there is an implementation -of `From<&str>` for `Box`. This means you can use `.into()` or -the `?` operator to convert your string into the correct error type. +Yet another hint: You can use the `map_err` or `or` methods of `Result` to +convert errors. + +Yet another hint: If you would like to propagate errors by using the `?` +operator in your solution, you might want to look at +https://doc.rust-lang.org/stable/rust-by-example/error/multiple_error_types/reenter_question_mark.html -If you're having trouble with using the `?` operator to convert an error string, -recall that `?` works to convert `Err(something)` into the appropriate error -type for returning from the function.""" +Challenge: Can you make the `TryFrom` implementations generic over many integer types?""" [[exercises]] name = "as_ref_mut" @@ -951,15 +974,3 @@ path = "exercises/conversions/as_ref_mut.rs" mode = "test" hint = """ Add AsRef as a trait bound to the functions.""" - -[[exercises]] -name = "from_str" -path = "exercises/conversions/from_str.rs" -mode = "test" -hint = """ -The implementation of FromStr should return an Ok with a Person object, -or an Err with an error if the string is not valid. -This is almost like the `try_from_into` exercise. - -If you're having trouble with returning the correct error type, see the -hints for try_from_into."""