From b7ddd09fab97fc96f032bc8c0b9e1a64e5ffbcdd Mon Sep 17 00:00:00 2001
From: Taylor Yu <tlyu@mit.edu>
Date: Wed, 9 Jun 2021 18:13:57 -0500
Subject: [PATCH] address review feedback

Adjust error text and naming to conform with best practices.
Use `map_err()` instead of `or()`. Wrap lower-level errors instead of
ignoring their details.

Also, don't "cheat" by bypassing the `new()` function in tests.

Fix a dangling reference in the try_from_into hints.
---
 exercises/error_handling/errors5.rs |  4 ++--
 exercises/error_handling/errors6.rs | 33 ++++++++++++++++++-----------
 info.toml                           | 21 ++++++++++--------
 3 files changed, 35 insertions(+), 23 deletions(-)

diff --git a/exercises/error_handling/errors5.rs b/exercises/error_handling/errors5.rs
index 1b11800..365a869 100644
--- a/exercises/error_handling/errors5.rs
+++ b/exercises/error_handling/errors5.rs
@@ -43,8 +43,8 @@ impl PositiveNonzeroInteger {
 impl fmt::Display for CreationError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         let description = match *self {
-            CreationError::Negative => "Number is negative",
-            CreationError::Zero => "Number is zero",
+            CreationError::Negative => "number is negative",
+            CreationError::Zero => "number is zero",
         };
         f.write_str(description)
     }
diff --git a/exercises/error_handling/errors6.rs b/exercises/error_handling/errors6.rs
index cee7250..0f6b27a 100644
--- a/exercises/error_handling/errors6.rs
+++ b/exercises/error_handling/errors6.rs
@@ -10,11 +10,20 @@
 
 // I AM NOT DONE
 
+use std::num::ParseIntError;
+
 // This is a custom error type that we will be using in `parse_pos_nonzero()`.
 #[derive(PartialEq, Debug)]
 enum ParsePosNonzeroError {
-    CreationError,
-    ParseIntError
+    Creation(CreationError),
+    ParseInt(ParseIntError)
+}
+
+impl ParsePosNonzeroError {
+    fn from_creation(err: CreationError) -> ParsePosNonzeroError {
+        ParsePosNonzeroError::Creation(err)
+    }
+    // TODO: add another error conversion function here.
 }
 
 fn parse_pos_nonzero(s: &str)
@@ -24,7 +33,7 @@ fn parse_pos_nonzero(s: &str)
     // when `parse()` returns an error.
     let x: i64 = s.parse().unwrap();
     PositiveNonzeroInteger::new(x)
-        .or(Err(ParsePosNonzeroError::CreationError))
+        .map_err(ParsePosNonzeroError::from_creation)
 }
 
 // Don't change anything below this line.
@@ -54,17 +63,18 @@ mod test {
 
     #[test]
     fn test_parse_error() {
-        assert_eq!(
+        // We can't construct a ParseIntError, so we have to pattern match.
+        assert!(matches!(
             parse_pos_nonzero("not a number"),
-            Err(ParsePosNonzeroError::ParseIntError)
-        );
+            Err(ParsePosNonzeroError::ParseInt(_))
+        ));
     }
 
     #[test]
     fn test_negative() {
         assert_eq!(
             parse_pos_nonzero("-555"),
-            Err(ParsePosNonzeroError::CreationError)
+            Err(ParsePosNonzeroError::Creation(CreationError::Negative))
         );
     }
 
@@ -72,15 +82,14 @@ mod test {
     fn test_zero() {
         assert_eq!(
             parse_pos_nonzero("0"),
-            Err(ParsePosNonzeroError::CreationError)
+            Err(ParsePosNonzeroError::Creation(CreationError::Zero))
         );
     }
 
     #[test]
     fn test_positive() {
-        assert_eq!(
-            parse_pos_nonzero("42"),
-            Ok(PositiveNonzeroInteger(42))
-        );
+        let x = PositiveNonzeroInteger::new(42);
+        assert!(x.is_ok());
+        assert_eq!(parse_pos_nonzero("42"), Ok(x.unwrap()));
     }
 }
diff --git a/info.toml b/info.toml
index 63eb78b..7695037 100644
--- a/info.toml
+++ b/info.toml
@@ -532,16 +532,19 @@ path = "exercises/error_handling/errors6.rs"
 mode = "test"
 hint = """
 This exercise uses a completed version of `PositiveNonzeroInteger` from
-the errors4.
+errors4.
 
-Below the TODO line, there is an example of using the `.or()` method
-on a `Result` to transform one type of error into another. Try using
-something similar on the `Result` from `parse()`. You might use the `?`
-operator to return early from the function, or you might use a `match`
-expression, or maybe there's another way!
+Below the line that TODO asks you to change, there is an example of using
+the `map_err()` method on a `Result` to transform one type of error into
+another. Try using something similar on the `Result` from `parse()`. You
+might use the `?` operator to return early from the function, or you might
+use a `match` expression, or maybe there's another way!
 
-Read more about `.or()` in the `std::result` documentation:
-https://doc.rust-lang.org/std/result/enum.Result.html#method.or"""
+You can create another function inside `impl ParsePosNonzeroError` to use
+with `map_err()`.
+
+Read more about `map_err()` in the `std::result` documentation:
+https://doc.rust-lang.org/std/result/enum.Result.html#method.map_err"""
 
 # Generics
 
@@ -927,7 +930,7 @@ 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 errorsn (or its hints) to remind
+You might want to look back at the exercise errors5 (or its hints) to remind
 yourself about how `Box<dyn Error>` works.
 
 If you're trying to return a string as an error, note that neither `str`