Skip to content

Improved Exaple Error Handling: Replaced all calls to unwrap() in examples with either the ? operator or expect() #839

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

Merged
merged 1 commit into from
Jan 13, 2019

Conversation

sunjay
Copy link
Contributor

@sunjay sunjay commented Jan 12, 2019

Part of #836

This PR modifies all the examples in the examples/ folder to use more idiomatic Rust error handling code. This PR is already quite big with quite a few (hopefully small and easy to review) changes, so converting the examples in the docs will be a separate PR later.

I manually went through and ran cargo run for each example, so hopefully they all compile and CI will pass.

Everything should still work exactly the same. This PR isn't meant to change any functionality. Just updating to the latest idioms.

Some notes:

  • All calls to unwrap() are completely gone from all examples
  • Majority of those calls are replaced with the ? operator where possible
  • Some calls could not be removed without a breaking change to the specs API, so expect() was used to keep this PR easy to merge (should be a future discussion?)
  • expect() was also added in some places where it is totally fine. For example, some of the previous calls to unwrap() were for cases we expect to be unreachable. In these cases expect() is just in case something goes really wrong. (no need for future discussion)
  • In addition to just calls to unwrap(), I also looked for panic!() calls and matching on Result types to find a bunch of places where code could either be improved or completely replaced with the ? operator

To make review as easy as possible, I avoided changing the structure of code as much as I could. That being said, I definitely noticed a lot of room for improvement in these examples. There is code that is starting to show its age and could be improved to be shorter, more idomatic, etc.

It's great to be able to give back to such a great library and I hope I'm able to contribute more in the future! Thanks for all your work on this! 😄

@Cobrand
Copy link
Member

Cobrand commented Jan 13, 2019

Looks good! Travis is happy as well, so I'm merging this!

@Cobrand Cobrand merged commit 37cf5f7 into Rust-SDL2:master Jan 13, 2019
@sunjay sunjay deleted the examples-no-unwrap branch January 17, 2019 05:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants