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

Returning `Result`s for a lot of rendering functions makes the api more complicated than needed. #512

Closed
expenses opened this Issue Nov 14, 2018 · 2 comments

Comments

Projects
None yet
2 participants
@expenses
Contributor

expenses commented Nov 14, 2018

Although some of these functions could fail (set_color cannot so I'm not sure why it returns a GameResult), I think it is generally better for game engines to just bite the bullet and panic if a buffer etc is unable to be created rather than let the user catch the error, especially if we're copying LÖVE.

@expenses expenses changed the title returning `Result`s for a lot of rendering functions makes the api more complicated than needed. Returning `Result`s for a lot of rendering functions makes the api more complicated than needed. Nov 14, 2018

@icefoxen

This comment has been minimized.

Contributor

icefoxen commented Nov 14, 2018

I sympathize but disagree. It's a matter of opinion when to panic and when to return Result, but returning Result has an overhead of a single ? character most of the time and I really dislike libraries that panic when they don't need to because the user then has almost no other choices but to suck it up and deal. Even if all you're doing is quitting anyway, there may be logging, state cleanup or crash reporting that you want to do before ending the program.

set_color returns GameResult mainly because everything else does. It doesn't exist at all in devel, so that inconsistency (or excessive consistency?) has been cleaned up.

@icefoxen icefoxen added the question label Nov 14, 2018

@icefoxen

This comment has been minimized.

Contributor

icefoxen commented Nov 14, 2018

Canonical response to things like this, in case the question ever comes up again: https://www.reddit.com/r/rust/comments/9x17hn/when_should_a_library_panic_vs_return_result/e9p5c9t

This question comes up so often, and there appears to be a lot of confusion around it. The choice is very rarely between "panic or Result," so phrasing an answer in terms of "use Result instead of panic" is a bit off. Panicking and Results solve two different problems.

The easy way to answer this is to state the conditions under which panicking is acceptable, since it is the more extreme behavior. Namely, if your library is the source of a panic, then one of the following should be true:

  • Your library has a bug.

  • Your library documents a precondition of a public API item that, when not met, causes a panic. Therefore, the user of your library has misused your library, and their code has a bug.

If neither of those is true, then outside special circumstances, you'll probably want to make one of those true. Which one to choose is sometimes judgment call, but either is valid.

If your Rust application panics in response to any user input, then the following should be true: your application has a bug, whether it be in a library or in the primary application code.

The bottom line is that if a panic surfaces to an end user, then we ought to consider that a bug somewhere.

@icefoxen icefoxen closed this Nov 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment