Hacker News Reader

Comment by Kwastie | Hacker News Reader

The author is missing the point. The fact that Optional can result in a nullpointer doesn't mean you should use in the same manner as null-checks.

You shouldn't replace:

  if(x == null)
  {
    y = x.doSomething();
  }

with

  if(optionalX.isPresent())
  {
    y = x.doSomething();
  }

You should replace it with:

  y = Optional.ofNullable(x)
    .map(ClassX::doSomething)
    .orElse(null);

View on HN

The author is missing the point.

Is he? The point is that in Java you are still able to treat x unsafely, while languages with stronger typing do not. E.g. in Haskell, if a function returns a Maybe a, it will always be a Just a or Nothing value. Moreover, such languages allow you to make non-exhaustive matching against all constructors a compiler error.

tl;dr: Haskell, Rust, et al. put the burden on the compiler. Java puts the burden to ensure safety on the programmer. (As can be witnessed in your snippet.)

>tl;dr: Haskell, Rust, et al. put the burden on the compiler. Java puts the burden to ensure safety on the programmer. (As can be witnessed in your snippet.)

Actually, (much to my disappointment as I'm just learning rust) you can just take an Option or Result and .unwrap() and the compiler won't complain at you for not checking it. For such a "safe" strongly-typed language, I'm surprised that so much new rust code does this when:

* the developer believes the Option/Result can never be None/Err (oh come on, just wait until you refactor your code a bit one day and miss a spot)

* the code is example code (I've had little luck finding "good" examples sprinkled about github)

* the developer is lazy (see above)

Allowing unsafe unwraps defeats a core purpose of rust, and the unwrap method shouldn't even exist in the API.

Just to clarify since `unsafe` is a special term in Rust: `unwrap` still checks for None and panics, it doesn't blindly assume it's valid.

That said, I think you're too dismissive of "can never fail" assumptions. There's tons of places where something is optional in general, but based on various invariants you know it won't be. For instance, if you successfully acquire the first element in an array, you know you can get the last.

If you legitimately believe that Option is always Some, I don't understand what you think you would otherwise put there? Gonna have your function return an Err if an internal invariant is broken? Gonna have callers actually check for that?

>Gonna have your function return an Err if an internal invariant is broken?

Absolutely you should return an error. Whether the caller wants to panic or handle it or print unicorns should be left up to the caller, not your function. Functions should not be expected to tear down the thread in case of an error. Nothing that panics should belong anywhere in exported code

This seems a bit too unpragmatic. Would you also require the user to explicitly handle:

* Index out of bounds on every array op

* Integer overflow on every arithmetic op

* OOM on every allocating op

Maybe if you're writing an ironclad RTOS for a critical system without any good redundancy? Otherwise these are such pervasive operations that most have accepted that they're not worth handling everywhere they happen. Requiring that really dilutes the value/meaning of errors.

Given this signature:

fn do_thing() -> Result<T, E>

There's a clear signal that there's legitimate error conditions that you probably want to think about today. If every function that accessed arrays, worked with integers, or allocated memory returned a Result, it would border on meaningless. It would be like if there was no Throwable/Exception distinction in Java. Everything would `throw Exception`, eliminating the value of even noting that something can fail.

I think the only way such a system could be tolerable is if the language in question had really good dependent type support (but that wouldn't handle the OOM issue -- which you can't even reliably handle on some systems).

Unwinding/crashing is valuable in a truly robust system, because it needs to handle crashes anyway. Might as well punt obscure problems to that level of the reliability system. (This is basically the basis of Erlang's task system, AFAIK)

>Requiring that really dilutes the value/meaning of errors.

No. You have already diluted the meaning of errors, and you want them elevated to _your_ standard.

>Index out of bounds on every array op

These are removed if you build a rust program with --release.

>Integer overflow on every arithmetic op

Add the Wrapping class if you expect overflow. Overflow _shouldnt_ normally happen on an Integer operation. It is a hardware error when it happens, and can cause massive pain-in-ass bugs when it happens unexpectedly.

I'd rather get errors when it does happen, rather then find out 6 months into a production run.

>OOM on every allocating op

C does this also.

What have I diluted the meaning of errors to?

Rust doesn't remove bounds checks in --release. It's wrapping that gets turned on in release. I'm not sure why you're distinguishing overflow as truly exceptional, as opposed to any other "this should never happen" error?

Also I don't think many C libraries that allocate expose that as a failure condition (I've certainly seen some which don't even check!)

>Also I don't think many C libraries that allocate expose that as a failure condition (I've certainly seen some which don't even check!)

This is a very common mistake in most C code. Malloc can fail, and it return NULL when it does.

IMO the more important example that Gankro failed to mention is division, where your language has to do something when the denominator is zero.

The idea is that Err is good if it's a recoverable error, but if it's not recoverable, you should panic!.

Most errors are recoverable. panic! is an antipattern in a library. Or at least, provide both a panic-ing and a non-panic-ing variant.

We specifically recommend against providing panicing and non-panicking. This causes horrible combinatoric explosions. There's very few exceptions to this (array indexing being the major exception). We do of course recommend non-panicking, but there's a few cases where the burden of checking for errors is too high (indexing, refcell).

We're actually in the midst of arguing whether to violate this convention and do it for RefCell: https://github.com/rust-lang/rust/issues/27733

Yeah, I was thinking of things like array access. The stuff that's on the wrong end of the cost/benefit ratio.

The problem is that a function cannot know whether or not the caller can recover from a particular error, so there's no point in making that distinction in the first place.

It's part of your API design, if they are recoverable or not. You should lean towards recoverable: if the caller can't recover, they can always do something with it. But some kinds of problems are non-recoverable.

Can you name one such unconditionally unrecoverable problem?

Say you have a library which has some kind of internal state, and the consistency of said internal state is important for memory safety, as other operations rely on it.

Using assert! to verify that your state is consistent is useful, in case you have a bug. But since it's all internal, it's not something that the caller can really recover from, either. It's not their fault, it's yours.

Well here's my issue with this explanation that I've heard a lot: the caller may or may not be able to recover from it, but the caller may still be doing something with the error. Maybe the caller doesn't want to expose the error to its caller, or maybe the caller wants to build a new message to explain the cause of your error. But to panic! takes away a lot of options for the caller.

Worst of all, it leads to untestable code, because the calling test case can't properly check the error against some known result.

Is this not a ridiculous thing for an application to check for? That the library it's using detected that it contains a bug?

> you can just take an Option or Result and .unwrap() and the compiler won't complain at you for not checking it.

Explicit vs implicit. In Rust/Haskell you are forced to do _something_ about the nullability. If you use unwrap/expect/fromJust you are explicitly acknowledging that you want it to panic if it fails.

On the other hand, in Java, you can get an NPE where you didn't expect it because nulls move around easily and there's nothing in the type system to prevent that. That's the difference.

> I'm surprised that so much new rust code does this

Also, in my experience, unwrap() in Rust is mostly confined to irrecoverable errors in applications, and example blog posts (where the focus is on getting something done and not worrying about good error handling).

> defeats a core purpose of rust

What purpose? Whatever unwrap provides is still possible via a match+panic. Which is _more_ explicit, but they're both still explicit.

> If you use unwrap/expect/fromJust you are explicitly acknowledging that you want it to panic if it fails.

If you use Optional.get() without Optional.isPresent(), how is that any different? It's not as nice as pattern decomposition, but it's still fundamentally the same, and on top of that, transform functions are provided so that most of the time you can do null-safe operations.

The difference is that Optional itself can be null, and in Java everything can be null.

It can be, and it's annoying, but that's still a step forward. It's trivial for a good static checker to treat all Optional references as if they were annotated with your favorite @Nonnull annotation.

But in Java you'd never use a library that calls System.exit when it reaches an unexpected null

See my other comment: https://news.ycombinator.com/item?id=10452408

In Rust you'd rarely write a library that panics on unexpected nulls.

(Also, panic != abort, yada yada)

>Actually, (much to my disappointment as I'm just learning rust) you can just take an Option or Result and .unwrap() and the compiler won't complain at you

unwrap() is a convenience method that still uses exhaustive pattern matching and will panic if no value is present. The developer is making a conscious decision to panic in a not-present scenario, and on a case-by-case basis. The compiler is still doing it's job. This is different than java, where a get() on a non-present value is a runtime violation.

The distinction is there but the runtime behavior is not remarkably different. This feels more like a philosophical argument with no real purpose, to be honest.

If you don't think it's an important distinction then your real beef is that the developer can choose to panic in certain scenarios, which of course is silly when put in context.

Allowing unsafe unwraps defeats a core purpose of rust,

Definitely, but you can also still do this in Haskell (fromJust). But it's better than nullable types since you explicitly have call an unsafe method. (Assuming that you have set non-exhaustive pattern matching to be a warning/error.)

unwrap() is not unsafe in Rust, and I don't think Haskell has anything similar?

Sorry for the confusion! I meant 'unsafe' as in partial (not safe for all inputs). Not as in Rust's unsafe keyword.

Ah, right. It's really important in the context of Rust. :) People sometimes claim that unwrap() violates memory safety, which isn't true.

The definition in Rust's documentation[0] was this:

    let x: Option<&str> = None;
    assert_eq!(x.unwrap(), "air"); // fails

The equivalent Haskell if unwrap throws a panic as the docs[0] seem to imply:

    λ> fromMaybe (error "panic!") Nothing
    *** Exception: panic!

You could also do something like this if you don't mind dealing with the wrapped boolean value:

    λ> fmap (== "air") (Just "air")
    Just True

and the failure case:

    λ> fmap (== "air") (Nothing)
    Nothing

0: https://doc.rust-lang.org/std/option/enum.Option.html#method...

Right, but it's still not unsafe. The stack unwinds, destructors get called.

And I meant some notion of 'safety', not fromMaybe/fromJust.

  > much to my disappointment as I'm just learning rust

Then prepare to be disappointed by Haskell as well, where the equivalent function is called `fromJust`. :P

Both languages discourage the use of these constructs, but they exist for good reasons.

> Both languages discourage the use of these constructs, but they exist for good reasons.

Curious, what are your good reasons for fromJust in Haskell?

I completely agree. unwrap() is a huge mistake in Rust. There are brilliant ideas implemented in Rust (primarily the borrow checker) but their advantages seem to be wiped out by the kludge of unwrap.

If we didn't provide it, someone would write it. (Actually, I suspect lots of libraries would just have it.) You can't force people to write good code.

This presupposes that the pattern that "unwrap" wraps is always a bad thing anyway, which it isn't. Sometimes the right thing to do is to panic your program if None is present. And sometimes the type system isn't sophisticated enough to express an invariant, and you need to use "unwrap" to work around it.

By the way, the borrow checker has nothing to do with unwrap().

Care to explain why? Whatever unwrap provides can be done with a match+panic. Which is more explicit, but that's just splitting hairs -- unwrap is pretty explicit anyway.

I understand the anger but at the same time I don't.

Overall unwrap() isn't good. Really it should be replaced by TODO WRITE ERROR HANDLING in 90% of its cases. That being said there are still good uses for it.

The main _safe_ use for unwrap() I find is when dealing with an iterator of Results/Options (which seems to happen a lot). The pattern:

      .filter( |x| x.is_some() )
      .map( |x| x.unwrap() )

Is safe, and I really don't know a more eloquent method of handling these operations.

In Swift you can solve this with a filterNone() method with an explicit type signature of [T?] -> [T], I assume that could also be implemented in Rust?

I'm not angry.

> Really it should be replaced by TODO WRITE ERROR HANDLING in 90% of its cases.

That's what the Rust community reads it as, mostly.

I've seen it occur very infrequently in libraries (aside from example/benchmark/test/mocking code, of course). When it does its usually for really out of whack errors like poisoned mutexes, crashed threads, etc (mind you, these are for unwrapping error values, not options, but it's almost the same thing). Or for cases where it's known to be valid due to invariants that can't be expressed in the normal way.

I did a quick grep of hyper, and the unwraps are almost all in "dev" code (tests/benchmarks/etc). The cases where they aren't are where there was an `is_some()` earlier (and due to some reasons it couldn't be restructured as a match, perhaps just because of rightward drift), or the "invariants" thing -- i.e. it often calls `<some url>.serialize_path().unwrap()`, which is OK, because hyper only ever deals with relative schemes (e.g. `http://` and `https://`, not `data:` or `mailto:` -- the latter two don't have paths), which is checked in the constructors, so this should NEVER panic. I didn't go through all the code, but in the main HTTP and HTTP2 code serialize_path was the only offender, which was one that was justified.

I see it more often in applications. In the following cases:

- Example applications in blog posts (because you want to explain something else, not spend too much time on error handling)

- Slightly less out of whack errors which are weird enough that they should close the app

- Normal errors which should close the app (e.g. "insufficient permissions" for a command line util, or a port-opening error on a network app). unwrap() prints out the error message so this is a very rudimentary form of error handling anyway.

- Laziness, or often with a TODO. Not too common.

So, it's mostly used in justifiable ways, and that's why it exists in the stdlib. It doesn't "wipe out" anything that the borrow checker provides (borrowchk and exhaustive match are two different things), and while it's a kludge if used often there are legitimate reasons to use it; enough reasons to have it in the stdlib.

> I really don't know a more eloquent method of handling these operations.

    .filter_map(|x| foo(x))

(but yes, in general there are situations in which you know an Option is unwrappable, e.g. the serialize example above)

> filter_map(|x| foo(x))

Thanks :D

I've been really struggling to get iterators in rust. I don't have a very strong functional background :|

I have an open PR with documentation for just Iterator alone that has a diff so big github can't expand it. Hope they help.

Note that anytime you see a closure in any language that just takes a single argument, calls a single function with that argument, and returns the value of that function, then you can eliminate the closure entirely by just passing the function instead.

So `filter_map(|x| foo(x))` can be written more simply as just `filter_map(foo)`.

I know this but I can never get my code to compile when doing this. I always seem to get huge type errors even when I know the code is correct.

     map(|x|foo(x)) //clean compile
     map(foo) //type error

The number of times I've had this happen has just lead to me giving up on simply just passing the lambda as a variable instead of wrapping it in another.

Curious. Can you provide a full code example so that I can try to determine what's causing that error?

Usually in these cases the culprit is autoderef/deref coercions.

I don't see anything in `|x| foo(x)` that could be causing `x` to be deref'd.

A deref coercion can happen there

https://github.com/Manishearth/rust-clippy/issues/116

Just make an issue?

I don't think it necessarily warrants an issue on the Rust repo, this is just for my own edification.

This is a dramatic overreaction. `unwrap` is a convenience method whose trivially-obvious implementation is four lines long. Were it not provided by the stdlib then we'd have a preponderance of bespoke implementations in the ecosystem all with different names, which would make this pattern much harder to distinguish and discourage.

(Not to mention that `unwrap` in no way subverts memory safety, which makes mentioning it in the same breath as the borrow checker puzzling.)

The point of the new Optional type in Java isn't to prevent NPEs, it's to make using the new Streams API cleaner. Since you can now flow through optional values in a stream you can ignore whether the stream contains optionals or not and only deal with them at the end. It's certainly possible that in a future version of Java the optional type might be paired with something like pattern matching to prevent NPEs with some syntax sugar, but that's not the reason that the Optional type exists today.

>Java puts the burden to ensure safety on the programmer. (As can be witnessed in your snippet.)

And that's OK too, if less than ideal. The Optional type serves as a reminder to use it differently.

As of Java 8, Java has pluggable type systems[1], some of them are more advanced than what Haskell provides (like true intersection unit types), some effect types (locks etc.) and more. So Java puts the burden on some very advanced -- but optional -- type systems.

[1]: http://types.cs.washington.edu/checker-framework/current/che...

Yes, but not in any sense that matters to 99.9% of Java programmers in the field.

That's still more than Haskell and Rust developers in the field... ;) In any case, as Checker matures and becomes more accessible, it will be marketed more and I expect higher adoption.

Not entirely true from what little I know about Haskell. Haskell might put MORE of the burden on the compiler but it doesn't put ALL of it there. The following generates a runtime exception. (not a compile time type error)

head []

Close, but not quite there. You should leap over tall buildings to make sure that you never have to set y to null.

In my code, I'm only dealing with null if a API gives me one. At that point, I take some sort of action to make absolutely sure I'm not passing it along to anyone else. This means using exceptions, the null object pattern, or Optional without a need to dereference. The goal is to prevent a proliferation of references that could be null and therefore have to be checked.

Imho the "you should replace it with" is the least readable code snippet of all 3.

(Also I assume the first is intended to be x != null)

I agree. Having programmed in java for the past 15 years, I can attest that NullPointerExceptions are a constant source of errors. The main benefit I've experienced with Optional is that it documents the fact that a method might return a null value. Without this, the client has to guess whether or not to add defensive null checks.

@Nullable already does that.

Of course, ClassX::doSomething must still be at least in principle prepared to handle nulls.

Reply on news.ycombinator.com