CODEWITHSUNDEEP
javajava-8enums
mitali
mitali

Reputation: 97

How to avoid NullPointer exception in Enums

I am writing an enum and making sure that the enum don't throw nullpointer exception, as it looks bad for the client who uses this enum. I am using Google enum, still it throws NPE if the input parameter mood is null

import com.google.common.base.Enums;
public enum KnowYourMoodEnum {
    ANGRY, SAD, HAPPY, FEELING_GOOD;
    
    private static final Set<KnowYourMoodEnum> HAPPY_MOOD_LIST = Sets.newHashSet(HAPPY, FEELING_GOOD);
    private static final Set<KnowYourMoodEnum> UPSET_MOOD_LIST = Sets.newHashSet(ANGRY, SAD);
    
    public static boolean isHappyMood(String mood) {
        return HAPPY_MOOD_LIST.stream()
                .anyMatch(x -> x.equals(Enums.getIfPresent(KnowYourMoodEnum.class, mood).orNull()));
    }
    
    public static boolean isUpsetMood(String mood) {
        return UPSET_MOOD_LIST.stream()
                .anyMatch(x -> x.equals(Enums.getIfPresent(KnowYourMoodEnum.class, mood).orNull()));
    }
    
}

Upvotes: 0

Views: 1632

Answers (3)

Holger
Holger

Reputation: 298389

Not every operation gets better with Streams. Instead of set.stream().anyMatch(x -> x.equals(…)) just use the simple and efficient set.contains(…). It’s simpler and instead of performing a linear search, it will do an efficient lookup. Not that it matters much for such small sets, but it’s still worth noting that the Stream API not only provides no benefit here, but a degradation of performance.

If you want the method to be tolerant towards inputs not matching an actual enum constant, it’s much simpler to initialize a Set of names in the first place.

public enum KnowYourMoodEnum {
    ANGRY, SAD, HAPPY, FEELING_GOOD;
    
    private static final Set<String> HAPPY_MOOD = Stream.of(HAPPY, FEELING_GOOD)
        .map(Enum::name).collect(Collectors.toSet());
    private static final Set<String> UPSET_MOOD = Stream.of(ANGRY, SAD)
        .map(Enum::name).collect(Collectors.toSet());

    public static boolean isHappyMood(String mood) {
        return HAPPY_MOOD.contains(mood);
    }

    public static boolean isUpsetMood(String mood) {
        return UPSET_MOOD.contains(mood);
    }
}

These methods do already evaluate to false for null or unknown names.

If you want the check to be case insensitive, you can use

public enum KnowYourMoodEnum {
    ANGRY, SAD, HAPPY, FEELING_GOOD;
    
    private static final Set<String> HAPPY_MOOD = Stream.of(HAPPY, FEELING_GOOD)
        .map(Enum::name).collect(Collectors.toCollection(
            () -> new TreeSet<>(String.CASE_INSENSITIVE_ORDER)));
    private static final Set<String> UPSET_MOOD = Stream.of(ANGRY, SAD)
        .map(Enum::name).collect(Collectors.toCollection(
            () -> new TreeSet<>(String.CASE_INSENSITIVE_ORDER)));

    public static boolean isHappyMood(String mood) {
        return mood != null && HAPPY_MOOD.contains(mood);
    }

    public static boolean isUpsetMood(String mood) {
        return mood != null && UPSET_MOOD.contains(mood);
    }
}

This will have O(log n) lookups instead of O(1), but that’s still better than an O(n) Stream operation and still not relevant for such small sets. Further, the methods are slightly more complicated with an explicit null check, but still much simpler than any attempt to bring in the Stream API at all costs.

And you don’t need a 3rd party library for such a simple task…

And by the way, don’t name variables …_LIST when they contain a Set.

Upvotes: 2

Nowhere Man
Nowhere Man

Reputation: 19565

Using Enums.getIfPresent seems to be redundant, it should be sufficient to compare (possibly ignoring case) the input mood to enum's name():

public static boolean isHappyMood(String mood) {
    return isInSet(mood, HAPPY_MOOD_LIST);
}

public static boolean isUpsetMood(String mood) {
    return isInSet(mood, UPSET_MOOD_LIST);
}
    
private static boolean isInSet(String mood, Set<KnowYourMoodEnum> set) {
    return set.stream().anyMatch(x -> x.name().equalsIgnoreCase(mood));
}

This is null-safe because name() is never null.

Upvotes: 2

chrylis -cautiouslyoptimistic-
chrylis -cautiouslyoptimistic-

Reputation: 77206

(1) It's entirely routine for APIs to specify "don't pass me a null or you get NPE". That makes sense here.

(2) If you don't want to do that, then if (mood == null) return false;.

(3) This is a complicated and indirect implementation. It seems like it would be simpler to have a property boolean happy on the enum.

Upvotes: 2

Related Questions