4 ms·

The problem is that Rust developers don't just use unwrap() when it should panic. I've seen plenty of "production grade" crates basically just unwrap because th

by cedws 4y ago

The problem is that Rust developers don't just use unwrap() when it should panic. I've seen plenty of "production grade" crates basically just unwrap because the author didn't know how to handle it gracefully or just wanted to get the code compiling, then forgot about it.

dymk 4y ago
I'm confused, you seem to be saying opposite things. You're saying authors don't use unwrap when it should panic, and in the next sentence, you say they use unwrap (which causes a panic) when the failure could be gracefully recovered.
- c7DJTLrn 4y ago
  "don't just"
  - OJFord 4y ago
    Heh, GP (I assume) and I initially parsed it as "don't just-use" (don't simply use) rather than "don't-just use" (don't only use).
  - dymk 4y ago
    Got it. OJFord has it right, my parens were in the wrong place.
dureuill 4y ago
not sure why you're getting downvoted. maybe the scope of what you're saying ('plenty of "production grade" crates unwrap [when they shouldn't]') calls for a reference? Or maybe because you're singling out Rust developers while the issue is certainly observed in all languages with similar mechanisms (see unchecked exceptions abuse in java, or aborting asserts abuse in C) Anyway i wouldn't say "plenty", but i did came across crates (parsers :/) that would unwrap on malformed input. the workaround is to encapsulate their use in a catch_unwind. for the record, i had a similar issue in a c++ lib where the author elected to abort on the unsupported input, so i'm somewhat thankful that the idiomatic mechanism is panic (which is recoverable if needs be) in Rust
ironmagma 4y ago
But luckily when that time does come to refactor the code to handle the error case, the Rust compiler does a lot to help you make sure it’s handled properly.
burntsushi 4y ago
That is a problem. Another similar problem is that programmers write buggy code. I don't see anything particularly special about 'unwrap()'. 'unwrap()' is a common manifestation of it. But as others have pointed out to you, this same phenomenon manifests everywhere and in other programming languages, but through different means.
- carlmr 4y ago
  This is why I write in a 2-pass manner quite often. In the first pass I use unwrap a lot to get the functionality going. In the second pass I search for all the unwrap and expect and take a moment to think about in what cases these may happen. This works for me really well to get a prototype going, but then have a solid program later. Because the unwrap/expect is so easy to search for, that you can really postpone the error handling until later.
librexpr 4y ago
Maybe it would have been a good idea to have two names for unwrap, one which would mean "I'm certain that this value will always be okay", and another which would mean "I'm taking a shortcut because I'm writing a script or just want it to compile for now". Maybe a longer name like "assert_valid()" for the one where you're sure it's okay. That might make it easier to find the places where shortcuts were taken and forgotten.
- deleted 4y ago
  [deleted]
- jeffrallen 4y ago
  Maybe if Rust code wasn't so hard to get working at all, then more of it would work correctly (including checking errors instead of giving up and unwrapping).
  - dureuill 4y ago
    not sure what you're referring to, but yes having a first iteration of code with unwrap here and there, and then a second one with proper error handling is a viable strategy. the "loudness" of unwrap makes it super easy to notice any leftover in code review
- r3un1 4y ago
  There is! `expect()`
  - sanderjd 4y ago
    I think this is a good answer. People should use `expect` for this "I'm sure this is fine" case, with descriptive text about why it's fine.
    - tomjakubowski 4y ago
      Or you can invert it and use `.expect("TODO add error handling")`
bern4444 4y ago
When writing a library, wouldn't the prudent thing to do be to return a Try/Either or have the method accept a function to invoke in case of an error (like an IOC solution)?
deathanatos 4y ago
Guilty as charged, and honestly, I wish I had a good mechanism/function call to separate "I'm just being lazy here, this is a TODO" from "actually, this should never happen. .map_err(|_| unimplemented!()).unwrap() or something. (I hope you get the gist of that, as I'm 99% sure that code doesn't compile. Error(E) -> !, Ok(T) -> T)
- burntsushi 4y ago
  Just write 'expect("FIXME")' or similar. :-)
  - deathanatos 4y ago
    I've thought about this, it just requires some effort, particular if you want the result to be grep-able. (And, in >1 person groups, then it requires coordination, although perhaps also all of those should be remove prior to committing…)
    - burntsushi 4y ago
      I think anything you do here is going to require some kind of effort. Personally, I think it would be better to stop thinking about 'unwrap()' as its own special thing. 'unwrap()' is just a common manifestation of something much more fundamental: runtime invariants. Think about those.
- dureuill 4y ago
  You could go: result.unwrap_or_else(|| todo !()) (unimplemented! is for missing functionality in a given version, todo! is for missing functionality during development) But using unwrap for these kinds of TODO is ok, code review won't let unjustified unwraps to pass
speed_spread 4y ago
This is why .unwrap() should have been named .or_panic(). The current wording makes it sound innocuous, even if you know what it does.

by cedws 4y ago

dymk 4y ago

I'm confused, you seem to be saying opposite things. You're saying authors don't use unwrap when it should panic, and in the next sentence, you say they use unwrap (which causes a panic) when the failure could be gracefully recovered.

c7DJTLrn 4y ago
"don't just"
- OJFord 4y ago
  Heh, GP (I assume) and I initially parsed it as "don't just-use" (don't simply use) rather than "don't-just use" (don't only use).
- dymk 4y ago
  Got it. OJFord has it right, my parens were in the wrong place.

dureuill 4y ago

not sure why you're getting downvoted. maybe the scope of what you're saying ('plenty of "production grade" crates unwrap [when they shouldn't]') calls for a reference? Or maybe because you're singling out Rust developers while the issue is certainly observed in all languages with similar mechanisms (see unchecked exceptions abuse in java, or aborting asserts abuse in C) Anyway i wouldn't say "plenty", but i did came across crates (parsers :/) that would unwrap on malformed input. the workaround is to encapsulate their use in a catch_unwind. for the record, i had a similar issue in a c++ lib where the author elected to abort on the unsupported input, so i'm somewhat thankful that the idiomatic mechanism is panic (which is recoverable if needs be) in Rust

ironmagma 4y ago

But luckily when that time does come to refactor the code to handle the error case, the Rust compiler does a lot to help you make sure it’s handled properly.

burntsushi 4y ago

That is a problem. Another similar problem is that programmers write buggy code. I don't see anything particularly special about 'unwrap()'. 'unwrap()' is a common manifestation of it. But as others have pointed out to you, this same phenomenon manifests everywhere and in other programming languages, but through different means.

carlmr 4y ago
This is why I write in a 2-pass manner quite often. In the first pass I use unwrap a lot to get the functionality going. In the second pass I search for all the unwrap and expect and take a moment to think about in what cases these may happen. This works for me really well to get a prototype going, but then have a solid program later. Because the unwrap/expect is so easy to search for, that you can really postpone the error handling until later.

librexpr 4y ago

Maybe it would have been a good idea to have two names for unwrap, one which would mean "I'm certain that this value will always be okay", and another which would mean "I'm taking a shortcut because I'm writing a script or just want it to compile for now". Maybe a longer name like "assert_valid()" for the one where you're sure it's okay. That might make it easier to find the places where shortcuts were taken and forgotten.

deleted 4y ago
[deleted]
jeffrallen 4y ago
Maybe if Rust code wasn't so hard to get working at all, then more of it would work correctly (including checking errors instead of giving up and unwrapping).
- dureuill 4y ago
  not sure what you're referring to, but yes having a first iteration of code with unwrap here and there, and then a second one with proper error handling is a viable strategy. the "loudness" of unwrap makes it super easy to notice any leftover in code review
r3un1 4y ago
There is! `expect()`
- sanderjd 4y ago
  I think this is a good answer. People should use `expect` for this "I'm sure this is fine" case, with descriptive text about why it's fine.
  - tomjakubowski 4y ago
    Or you can invert it and use `.expect("TODO add error handling")`

bern4444 4y ago

When writing a library, wouldn't the prudent thing to do be to return a Try/Either or have the method accept a function to invoke in case of an error (like an IOC solution)?

deathanatos 4y ago

Guilty as charged, and honestly, I wish I had a good mechanism/function call to separate "I'm just being lazy here, this is a TODO" from "actually, this should never happen. .map_err(|_| unimplemented!()).unwrap() or something. (I hope you get the gist of that, as I'm 99% sure that code doesn't compile. Error(E) -> !, Ok(T) -> T)

burntsushi 4y ago
Just write 'expect("FIXME")' or similar. :-)
- deathanatos 4y ago
  I've thought about this, it just requires some effort, particular if you want the result to be grep-able. (And, in >1 person groups, then it requires coordination, although perhaps also all of those should be remove prior to committing…)
  - burntsushi 4y ago
    I think anything you do here is going to require some kind of effort. Personally, I think it would be better to stop thinking about 'unwrap()' as its own special thing. 'unwrap()' is just a common manifestation of something much more fundamental: runtime invariants. Think about those.
dureuill 4y ago
You could go: result.unwrap_or_else(|| todo !()) (unimplemented! is for missing functionality in a given version, todo! is for missing functionality during development) But using unwrap for these kinds of TODO is ok, code review won't let unjustified unwraps to pass

speed_spread 4y ago

This is why .unwrap() should have been named .or_panic(). The current wording makes it sound innocuous, even if you know what it does.