CODEWITHSUNDEEP
javacoding-style
Luigi Massa Gallerano
Luigi Massa Gallerano

Reputation: 2357

Use static final unmodifiable Set as constant for checking variables value

I've often run through a validation pattern where, to be valid, some variables must contain one of a prefixed number of values. 

PSEUDO CODE:
    IF x == CONSTANT_1 || X == CONSTANT_2 || ... || x == CONSTANT_N
    THEN X is valid

In order to avoid the chain of OR terms, I created a static final unmodifiable set, which contains all the constants:

public final static String CONSTANT_1 = *value* ;
public final static String CONSTANT_2 = *value* ;
...
public final static String CONSTANT_N = *value* ;

public final static Set SET_OF_CONSTANTS = Collections.unmodifiableSet(new HashSet(){
    private static final long serialVersionUID = 1L;
    {
        add(CONSTANT_1); 
        add(CONSTANT_2);
        ...
        add(CONSTANT_3);
    }
});

And I perform the check in the following way:

if(!SET_OF_CONSTANTS.contains(x)){ 
    //X NOT VALID 
}

I'd like to know if this is a good programming practice, if there are any alternatives, and if it's true that using a Hash Table query (O(1) in theory) instead of the OR terms-chain improves performance and maybe also code-readability.

Upvotes: 8

Views: 14584

Answers (2)

ruben056
ruben056

Reputation: 112

Another (shorter) way would be to use Set.of : 

public final static Set SET_OF_CONSTANTS = Collections.unmodifiableSet(
    Set.of(CONSTANT_1, CONSTANT_2, CONSTANT_3));

Upvotes: 2

NPE
NPE

Reputation: 500327

Overall, I think this is very good style.

There's not a huge difference, but I'd personally define SET_OF_CONSTANTS like so:

      public final static String CONSTANT_1 = "*value*";
      public final static String CONSTANT_2 = "*value*";
              ...
      public final static String CONSTANT_N = "*value*";

      public final static Set<String> SET_OF_CONSTANTS = Collections.unmodifiableSet(
        new HashSet<String>(Arrays.asList(
              CONSTANT_1, 
              CONSTANT_2,
                      ...
              CONSTANT_N
              )));

It's not entirely clear to me whether you even need the separate CONSTANT_1 constants, or whether you can simply fold the values into SET_OF_CONSTANTS.

As far as performance goes, I would not start optimizing anything until I've profiled the code on real data.

Finally, note that when x is a string, the following is probably incorrect:

IF x == CONSTANT_1 || x == CONSTANT_2 || ... || x == CONSTANT_N

Here, the == should probably be replaced with calls to equals().

Upvotes: 17

Related Questions