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

errors6 feedback #2157

Closed
mnshdw opened this issue Nov 13, 2024 · 2 comments
Closed

errors6 feedback #2157

mnshdw opened this issue Nov 13, 2024 · 2 comments
Labels
A-exercises Area: Exercises C-enhancement Category: Enhancement P-medium Priority: Medium

Comments

@mnshdw
Copy link
Contributor

mnshdw commented Nov 13, 2024

errors6 suggests adding a function to map the error, that has then to be called with map_err.

    // TODO: Add another error conversion function here.
    // fn from_parse_int(???) -> Self { ??? }
        let x: i64 = s.parse().map_err(ParsePosNonzeroError::from_parse_int)?;
        //                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

It seems to me that it would be more idiomatic to implement From<ParseIntError>, so we could get away with a simpler s.parse()?.

Is there a specific reason this is not the suggested solution here?

@mo8it
Copy link
Contributor

mo8it commented Nov 13, 2024

It seems to me that it would be more idiomatic to implement From<ParseIntError>, so we could get away with a simpler s.parse()?

True. But errors6 is before the exercises about traits and conversions with From.

I think that we should add the implementation of From<ParseIntError> as an additional and preferred way to the solution. A comment should mention that traits like From are dealt with in later exercises.

Do you want to create a PR?

@mo8it mo8it added C-enhancement Category: Enhancement A-exercises Area: Exercises P-medium Priority: Medium labels Nov 13, 2024
@mnshdw
Copy link
Contributor Author

mnshdw commented Nov 13, 2024

But errors6 is before the exercises about traits and conversions with From.

Ah! Good point.

Do you want to create a PR?

Done. #2158

@mo8it mo8it closed this as completed Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-exercises Area: Exercises C-enhancement Category: Enhancement P-medium Priority: Medium
Projects
None yet
Development

No branches or pull requests

2 participants