Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

Pet peeve: unwrap() should be deprecated and renamed or_panic(). More consistent with the rest of stdlib methods and appropriately scarier.


That's kind of what I'm saying with the blind spot comment. The words "unwrap" and "expect" should be just as much a scary red flag as the word "panic", but for some reason it seems a lot of people don't see them that way.


Even in lowly Java, they later added to Optional the orElseThrow() method since the name of the get() method did not connote the impact of unwrapping an empty Optional.


I've found both methods very useful. I'm using `get()` when I've checked that the value is present and I don't expect any exceptions. I'm using `orElseThrow()` when I actually expect that value can be absent and throwing is fine. Something like

    if (userOpt.isPresent()) {
      var user = userOpt.get();
      var accountOpt = accountRepository.selectAccountOpt(user.getId());
      var account = accountOpt.orElseThrow();
    }
Idea checks it by default and highlights if I've used `get()` without previous check. It's not forced at compiler level, but it's good enough for me.


While the `Optional` API is generally pretty inconvenient (compared e.g. to Kotlin), it does offer the more precise `ifPresent`.


Java lambdas are terrible so I usually avoid them. There's no reason to invent new methods, when language already has corresponding statements.


A lot of stuff should be done about the awful unwrap family of methods.

A few ideas:

- It should not compile in production Rust code

- It should only be usable within unsafe blocks

- It should require explicit "safe" annotation from the engineer. Though this is subject to drift and become erroneous.

- It should be possible to ban the use of unsafe in dependencies and transitive dependencies within Cargo.


The `unsafe` keyword means something specific in Rust, and panicking isn't unsafe by Rust's definition. Sometimes avoiding partial functions just isn't feasible, and an unwrap (or whatever you want to call the method) is a way of providing a (runtime-checked) proof to the compiler that the function is actually total.


Panics should be explicit, not implicit.

unwrap() should effectively work as a Result<> where the user must manually invoke a panic in the failure branch. Make special syntax if a match and panic is too much boilerplate.

This is like an implicit null pointer exception that cannot be statically guarded against.

I want a way to statically block any crates doing this from my dependency chain.


unwrap is explicit.


What happens if a dependency of mine unwrap()s or expect()s something that is an error or isn't there? What should I do?

How was I informed as a user? It's not in the type signature.

Sounds like I get to indeterminately crash at runtime and have a fun time debugging.


That would require an effects system[0] like Koka's[1]. Then one could not only express the absence of panics but also allocations, infinite loops and various other undesirable effects within some call-trees. This is a desirable feature, but an enormous undertaking.

Absent that there are hacks like no_panic[2]

[0] https://blog.yoshuawuyts.com/extending-rusts-effect-system/ [1] https://koka-lang.github.io/koka/doc/book.html#why-effects [2] https://crates.io/crates/no-panic


Same thing that would happen if it did a match statement and panicked. The problem is the panic, not the unwrap.

I don’t think you can ever completely eliminate panics, because there are always going to be some assumptions in code that will be surprisingly violated, because bugs exist. What if the heap allocator discovers the heap is corrupted? What if you reference memory that’s paged out and the disk is offline? (That one’s probably not turned into a panic, but it’s the same principle.)


Not explicit enough, apparently.


Not sure what you're saying with the "work as a Result<>" part...unwrap is a method on Result. I think you're just saying the unwrap/expect methods should be eliminated?


Than they are going to write None | Err => yolo() that has the same impact. It is not the syntax or the semantic meaning is the problem here but the fact that there is no monitoring around the elevated error counts after a deployment.

Software engineers tend to get stuck in software problems and thinking that everything should be fixed in code. In reality there are many things outside of the code that you can do to operate unreliable components safely.


Exactly. People are very hung up on "unwrap" but even if it wasn't there at all, you will have devs just manually writing the match. Or, even more likely, using a trivial 'unwrap!" macro.

There's also an assumption here that if the unwrap wasn't there, the caller would have handled the error properly. But if this isn't part of some common library at CF, then chances are the caller is the same person who wrote the panicking function in the first place. So if a new error variant they introduced was returned they'd probably still abort the thread either by panicking at that point or breaking out of the thread's processing loop.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: