Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Use of Optional in a map

Ok before I start explaining my question I want you to know that I know about the design idea behind Optional and that it isn't intended to be used in fields or collections, but I have programmed a lot in Kotlin currently and really dislike using null.

So I have a Node based editor like in the Unreal Engine and each node has ConnectionBoxes, which can be either free or are occupied by a Connection.

So there are different ways to express this one of which is using a map that maps each ConnectionBox to a Connection like:

Map<ConnectionBox, Connection> connectionEndPoints;

and Connection could be null if the ConnectionBox is free. I don't like this, since an other developer doesn't know about the function of this Map and that it may return null for an existing ConnectionBox.

So I am tempted to use a:

Map<ConnectionBox, Optional<Connection>> connectionEndPoints;

which displays the intend much better and you can even read it like:
"This ConnectionBox may have a Connection attached."

My question is: Why should I not do this, even though it shows the intend much more clearly and prevents NPEs. Every SO-thread and every blog post says this is bad style and even the compiler say that I shouldn't do it with a warning.


On request here is a SO-thread that discourages use of Optional as a field or Collection value: Uses for Optional

And here is the warning (as it turns out it is a warning from IntelliJ):
Warning: Optional used as type for field {fieldName}


Ok after recommending that the Connection reference should lie within the ConnectioBox the question just shifts.

Why is:

class ConnectionBox {
    Optional<Connection> connection;
    ...

worse than

class ConnectionBox {
    Connection connection; //may be null
    ...

Unless I make a comment you can't see that you may run into a NPE and I dislike comments explaining code that could explain itself.

like image 358
Jhonny007 Avatar asked Oct 21 '16 12:10

Jhonny007


1 Answers

Edit

After watching Stuart Marks' (who works for the core libraries team in the JDK group at Oracle) talk "Optional – The Mother of All Bikesheds" from Devoxx 2016, you should jump to 54:04:

Why Not Use Optional in Fields?

  • More a style issue than a correctness issue
    • usually there's a better way to model absence of a value
    • use of Optional in fields often arises from slavish desire to eliminate nullable fields
    • remember, eliminating nulls isn't a goal of Optional
  • Using Optional in fields...
    • creates another object for every field
    • introduces a dependent load from memory on every field read
    • clutters up your code
    • to what benefit? ability to chain methods?

Original Post

According to IntelliJ's inspector (Preferences > Editor > Inspections > 'Optional' used as field or parameter type):

Optional was designed to provide a limited mechanism for library method return types where there needed to be a clear way to represent "no result". Using a field with type java.util.Optional is also problematic if the class needs to be Serializable, which java.util.Optional is not.

This also applies to collections in case you have to serialize them. Furthermore, have a look at these links:

  • Java 8 Optional: What's the Point?

    So to recap - in an attempt to get rid of NullPointerExceptions we have a new class that:

    • Throws NullPointerExceptions
    • Can itself be null, causing a NullPointerException
    • Increases heap size
    • Makes debugging more difficult
    • Makes serializing objects, say as an XML or JSON for an external client, much more difficult
  • Why java.util.Optional is broken

    The final irony is that by attempting to discourage nulls, the authors of this class have actually encouraged its use. I'm sure there are a few who will be tempted to simply return null from their functions in order to "avoid creating an unnecessary and expensive Optional reference", rather than using the correct types and combinators.

If you care about readability, you could also use @Nullable (available in Eclipse as well as in IntelliJ):

class ConnectionBox {
    @Nullable
    Connection connection;
    // ...
}

Alternatively, you can create an optional getter:

class ConnectionBox {
    Connection connection;
    // ...
    Optional<Connection> getConnection() {
        return Optional.ofNullable(connection);
    }
}
like image 95
beatngu13 Avatar answered Oct 23 '22 11:10

beatngu13