Adding catch_panic anti-pattern#22
Adding catch_panic anti-pattern#22liamzdenek wants to merge 2 commits intorust-unofficial:masterfrom
Conversation
README.md
Outdated
| ### Anti-patterns | ||
|
|
||
| * TODO thread + catch_panic for exceptions | ||
| * [thread + catch_panic for exceptions](anti_patterns/catch_panic.md) |
There was a problem hiding this comment.
Could you change the title here to match the title in the description
|
@liamzdenek huge apologies for letting this and your other PR slip off my radar - it got pushed down my notifications and I forgot about it. I'll get some feedback for you before the end of the week. cc @cbreeden, @lfairy if you have time to comment here it would be appreciated. |
|
|
||
| // the program continues running despite the panic | ||
| println!("potential error: {:?}", result); | ||
| assert!(result.is_err()); |
There was a problem hiding this comment.
The example could be improved by adding a function and which panics and catching the panic in the caller, then matching the Result. Describing the example you could show how by returning a Result, the Result-ness of the function is described in the signature.
|
|
||
| Expected errors should not result in stack unwinding. Instead, expected errors | ||
| should be handled through the Result and Option types. [The Rust Book's chapter | ||
| on Error Handling](https://doc.rust-lang.org/book/error-handling.html) elaborates further on this. |
There was a problem hiding this comment.
Could you add why stack unwinding is bad? And perhaps the other disadvantages of using catch_unwind? Also, where is it appropriate to use catch_unwind
There was a problem hiding this comment.
Also: since Result::unwrap() converts the error to a string, it's harder to distinguish between different kinds of errors than if we had matched the result directly.
|
Ping, @liamzdenek are you still alive? |
|
@liamzdenek any updates on this? |
|
Closing due to inactivity. Further additions to the PR can be made in #109. |
No description provided.