Java Optional questions, am I doing this right?

Question

I've had a "Bad habit" of tossing null into places, such as enumerators when something doesn't exist.

Example:

private enum Foo {
    NULL(1, null, 2),
    NOT_NULL(3, new Bar(), 4);

    private int a, c;
    private Bar b;

    Foo(int a, Bar b, int c) {
        this.a = a;
        this.b = b;
        this.c = c;
    }
}

So now I'm trying to convert my code to use Optional<T> Like everyone is suggesting, but I'm not sure if I'm doing it correctly.

Here's my code (Trimmed enum):

public static enum Difficulty { 
    EASY, MEDIUM, HARD
}

public static enum SlayerTasks {
    NONE(0, Optional.empty(), Optional.empty(), Optional.empty()),
    NPC(1, Optional.of(Difficulty.EASY), Optional.of("That one place."), Optional.of(1));

    private int taskId;
    private Optional<Difficulty> difficulty;
    private Optional<String> location;
    private Optional<Integer> npcId;

    SlayerTasks(int taskId, Optional<Difficulty> difficulty, Optional<String> location, Optional<Integer> npcId) {
        this.taskId = taskId;
        this.difficulty = difficulty;
        this.location = location;
        this.npcId = npcId;
    }

    public int getTaskId() {
        return taskId;
    }

    public Difficulty getDifficulty() {
        return difficulty.get();
    }

    public String getLocation() {
        return location.get();
    }

    public int getNpcId() {
        return npcId.get();
    }
}

What's bothering me is the documentation referring to #get() found here where it states:

If a value is present in this Optional, returns the value, otherwise throws NoSuchElementException.

So, I figured that in order to prevent this I would wrap the getter in #isPresent(), but then I couldn't figure out how to return empty.

Is this the correct way to do things, or am I missing something? I'm not looking for a "Fix", I'm looking for information on efficiency and proper practices.

If you are calling Optional.get() you are probably not doing it right. — Brian Goetz, Nov 24 '14 at 11:42

chiastic-security · Accepted Answer · 2014-11-24T10:55:56.573

4

You need to ask yourself what you want your getter to do if there's nothing to return.

There are only really four options:

Return a null (but then you're back to what you were trying to avoid);
Have your getter return an Optional<T> instead of a T;
Return a default value if there's nothing set;
Throw an exception.

I'd go with 2 unless there's a very clearly right answer for what the default should be. 4 is appropriate only if the client code should always know whether there's something there and only ask for it if there is (which would be unusual, though not impossible).

edited Nov 24 '14 at 10:55

answered Nov 24 '14 at 10:29

chiastic-security

20,430
4
39
67

A quick offtopic question, do you know how to get an EnumSet<> from a steam().filter()? ex: `private static final EnumSet ELEMENTS = EnumSet.of(SlayerTasks.class); public static EnumSet byType(Difficulty difficulty) { return ?; }` – Hobbyist Nov 24 '14 at 11:08
2

@Christian.tucker: `collect(Collectors.toCollection(()->EnumSet.noneOf(SlayerTasks.class)))` – Holger Nov 24 '14 at 11:30

score 2 · Answer 2 · answered Nov 24 '14 at 10:25

2

You can replace location.get() with location.orElse("SomeDefaultValue") if you wish to avoid the exception. This allows you to return a default value when the Optional is empty.

answered Nov 24 '14 at 10:25

Eran

387,369
54
702
768

score 1 · Answer 3 · answered Nov 24 '14 at 10:30

IMO, if you're implementing your logic using 'maybe' monad (Optional values) you should stick to Optional object and toss it around, extracting the wrapped value only if its required.

To modify undelying value you can use Optional.ifPresent(), Optional.map() or Optional.flatMap() methods, e.g.

Optional<Difficulty> difficulty = NPC.getDifficulty();
difficulty.ifPresent(diff -> { /* do comething here ... */ });

Java Optional questions, am I doing this right?

3 Answers3

Linked