Hacker News Reader

Comment by nv-vn | Hacker News Reader

Having the get() method makes the entire purpose of Optional useless. All it does is make your null-checking code even more verbose. If you need to extract the value, the orElse method should be suitable on all cases. Deprecating get() should be an instant decision, since other than backwards compatibility it has nothing to add to the Optional class. Including it in the first place was as much of a mistake as the null pointer.

View on HN

There is a big difference between null and .get()

With null, the actual error can show up somewhere completely different in your code making it hard to trace where the null value came from. With .get() the errors happens at exactly the point where I told the compiler: "trust me, this isn't empty!" but was wrong.

"making it hard to trace where the null value came from"

The pervasive lack of defensive programming slays me.

Defensive programming in the form of checking all parameters and all return values from library methods for null? Sure, good idea. But then you need a way to distinguish between things that should never be null and things that genuinely might not be present. So Optional is one of the pieces you need to enable this kind of style.

"distinguish between things that should never be null and ... might not be present"

My strategy is to use Enums. And no autoboxing. And use NullObjects.

Enums don't have (dynamic) data members, do they? Sometimes you want a representation of e.g. "user's email address, or nothing if they haven't set one". More, you want that often enough that it's worth having a single generic way to represent it rather than having to construct a custom NullObject for each case.

The traditional options are Java are:

1. Assume parameters aren't null, and then everything breaks and you fix it.

2. Assume ANY object can be null, and check them all, in every single method, and create correct handling in each case. Basically no one does this.

3. Try to figure out which things can be null or not depending on context. Sometimes you are still wrong, and get NPEs. This is what people actually do.

By defensive programming, do you mean #2? Because that kinda sucks compared to just using Optionals.

Yeah, even when using get() with Optional it's a great way to give semantics to the function that require you to stop for a second. Annotations don't really cut it for most IDEs/Lints.

#1 is standard practice

#2 Optionals are syntactic sugar for null checks.

#1 is what most programmers do, basically fragile code that can break at any time.

#2 is what I do. I'm not a great programmer, but keeping this sort of discipline means that my code has fewer trivial (and time-wasting) bugs.

I do option (2) for every parameter in every non-private method in my Java code. I have a template in my IDE to write the check; all I do is invoke the template, type the variable name, and hit enter. I even document the throw in the javadoc comments. It's not hard, and it's the right thing to do.

Java code gets littered with null checks. It's noise.

Sometimes, you _know_ that something will be valid. Rust has an unwrap method that does the same as get does, and legitimate uses of it come in cases like this:

    use std::net::Ipv4Addr;

    fn main() {
        let addr: Ipv4Addr = "0.0.0.0".parse().unwrap();
    }

Here, I _know_ that the call to `parse()` will always succeed. More verbose handling isn't helpful.

(We almost changed unwrap to assert to make it even more clear, but not everyone agreed that it was more clear)

Sounds like you want IP address literals (or more generally custom literals). If it's safe because everything's static, then, uh, make it static.

In this specific case, sure. But it's not really about this specific case; it's about the general idea of "I know that this won't fail, even though you, the compiler, do not." Unless you have full-on dependent types, (and even then), there's going to be times where that's true; it's the nature of type systems. They have to be conservative, there will always be valid programs that are not expressible.

I feel like that's just a special case of casting and so should be handled by casting. You can always cast Optional to Some, can't you? It's verbose and dangerous-looking but it should be verbose and dangerous-looking in my book.

It depends on the language. In a language like Rust, direct casts mean "reinterpret these bits", and the two are represented differently, so that wouldn't work. In a less low-level language, then sure, maybe a cast would work for this case. I'm not sure that it's any more verbose or scary looking, though.

> In a language like Rust, direct casts mean "reinterpret these bits", and the two are represented differently, so that wouldn't work.

What's the representation of an optional then? Doesn't make use of some generic concept like subclasses or unions? Surely you have a generic syntax for "assume this union is this particular member" or "assume this base class is this particular subclass" or the like?

> I'm not sure that it's any more verbose or scary looking, though.

I guess the point is that I want all cast-like operations to look alike (or at least to have a way to consistently distinguish them from ordinary methods) so that I can flag them all up consistently in code reviews, linters etc.

It's a tagged union. Sometimes the tag can be elided, sometimes not. If you did a direct cast, you'd lose the tag. More specifically though, layout is undefined, so you don't know if the tag is first or last, strictly speaking.

Lints for unwrap already exist :)

> It's a tagged union. Sometimes the tag can be elided, sometimes not. If you did a direct cast, you'd lose the tag. More specifically though, layout is undefined, so you don't know if the tag is first or last, strictly speaking.

Do you not have a generic syntax for unwrapping a tagged union, forcibly assuming that it's one particular component? That seems like something users would want.

That is exactly what unwrap() is...

(More specifically, it's a method which uses `match` internally to do this)

Ok, so I'm advocating forcing users to always use match directly if they want to do that. That helps ensure unsafe code looks unsafe.

Maybe your issue is that the `get` method in Java sounds pretty innocuous, but in Rust the `unwrap` method is notorious. Every Rust user knows that it can panic. Requiring a match would increase verbosity and decrease clarity.

Why add a language feature when a library feature does the same thing?

Hey, it worked for try!()/?

What if we decide that adding IP literals to the language isn't worth the complexity?

In practice, "add a new language feature" isn't a real solution for programmers who need to get the job done. Static type systems always have limitations. You need to be able to circumvent the type system if it isn't expressive enough for your use case.

Sure - you need casts or a similar feature. You don't need standard library methods that do casts, though, and having them invites trouble IMO.

Library methods are preferable to added language complexity.

You need the language functionality of casting anyway, no? Best that there's one and only one way to step around the type system.

No, you need pattern matching, which does not offer a convenient way to write Option.get().

> Having the get() method makes the entire purpose of Optional useless.

It doesn't quite, though it is way too short and convenient.

> If you need to extract the value, the orElse method should be suitable on all cases.

orElse is incorrect if you have specific expectations. get is an assertion, and used in that manner it's perfectly sensible and orElse is not. Plus orElse can send the wrong signal.

> Including it in the first place was as much of a mistake as the null pointer.

The only language I know which has optionals and doesn't have something equivalent to .get() is Elm, and even there you can trivially pattern-match and call Debug.crash in the Nothing case.

You should never have concrete expectations or make assertions on an unknown, that's the entire point of any implementation of Option.

Option.get leads to false confidence. I've had many arguments over use .get (in Scala) where another developers argument is "we check that it's defined higher up in the code". At that point a harmless looking refactor by someone else can introduce NPEs.

> You should never have concrete expectations or make assertions on an unknown

As long as Optional#isPresent exists, you can have "concrete expectations" of an optional's content (or lack thereof).

> At that point a harmless looking refactor by someone else can introduce NPEs.

The fundamental problem is not .get, it's that you're qualifying code changes involving partial function as "harmless-looking" and "refactoring"

Option.get is important. It lets you work around the type system in cases where the type system wasn't expressive enough. Given that all type systems are imprecise, this functionality is essential.

I'm all for strong type systems, but I also acknowledge their limitations.

If someone has managed to bork the code with a harmless looking refactor, an NPE is what you want. The code is no longer working as designed and needs to be fixed.

Remember that in this scenario the alternative to an NPE is that the program doesn't even compile (because it won't type check).

I disagree. I'd rather getWhenPresent than orElse if the code has nothing to do with a missing value. If someone needs the value and uses orElse without isPresent, their code will break in new and unexpected ways rather than in the obvious one.

(An even better solution is SWIFT's approach to nullable types, but that boat has sailed; intellij etc could however implement a static checker for bare Optional-getting)

How does orElse() cause new problems and/or break code? It is the same as get but forces you to think about the not available option...?

I think the options orElse/orElseGet and orElseThrow are enough to replace every get() method, and they'll force you to think about the missing scenario.

If you want to do something in case it is present, use ifPresent(lambda).

In case you want to return something, use orElse/orElseGet or orElseThrow, or just return the Optional itself.

> I think the options orElse/orElseGet and orElseThrow are enough to replace every get() method, and they'll force you to think about the missing scenario.

orElseThrow doesn't force you to think about the missing scenario, only to think about which exception you want to throw, which in many case you don't care for.

If orElseThrow had an override throwing NoSuchElementException by default it would be a perfect replacement, alas it does not.

Of course it does, you actively choose to throw something! That is, in some cases, perfectly acceptable. All three methods tell you there is something else in case you don't have a value in the optional. get() doesn't.

Java Optional was inspired by Scala Option was it not? And I haven't seen anyone complain about Scala Option having exactly the same get() method for ages.

I think it has to do with the mindset of people working with Java vs Scala. Specifically, Scala folks are taught to reach for functional concepts(map, flatMap, etc) first whereas these concepts are still new to Java land.

It does, but the scala community is very familiar with the functional ("lambda") approach of leaving values in an optional context while transforming it.

The issue here stems from the existing community's culture, one especially full of trivially safe getters.

Well, maybe they need to learn the lesson the hard way, because when they start doing `foo.getBar().map(this::toString).orElse("Bar not found");` they will realize (hopefully) that this looks much more like Scala than classic Java.

I believe less Scala developers do "IDE-driven-development", where they just briefly look at the auto-complete to find a methods that vaguely appears to do what they want.

The ScalaZ folks use their own Maybe type instead so as not to have a get() method. Wartremover makes Option#get() a compile failure. Where I work "Option#get" is an automatic failure in codereview. For political reasons I doubt it will ever be removed in the Scala standard library, but there is certainly a substantial contingent that objects to it.

a) he doesn't propose to deprecate the semantics of get(), just change the name.

b) get() is useful and less verbose for cases where other logic has proven that the get() will succeed.

    boolean allPresent = a.isPresent() && b.isPresent()
    if (allPresent){
        doSomething(a.get())
    }

edit: just noticed this example is the same idea merb posted earlier.

Reply on news.ycombinator.com