tags:

views:

1233

answers:

8

I'm trying to lookup against an Enum set, knowing that there will often be a non-match which throws an exception: I would like to check the value exists before performing the lookup to avoid the exceptions. My enum looks something like this:

public enum Fruit {
 APPLE("apple"),
 ORANGE("orange");
    ;
    private final String fruitname;
    Fruit(String fruitname) {
        this.fruitname = fruitname;
    }
    public String fruitname() {return fruitname;}
}

and I want to check if, say, "banana" is one of my enum values before attempting to use the relevant enum. I could iterate through the permissible values comparing my string to

Fruit.values()[i].fruitname

but I'd like to be able to do something like (pseduo-code):

if (Fruit.values().contains(myStringHere)) {...

Is that possible? Should I be using something else entirely (Arrays? Maps?)?

EDIT: in the end I've gone with NawaMan's suggestion, but thanks to everyone for all the helpful input.

+4  A: 

When I do this I usually graft it onto my enum class.

public enum Fruit {
        APPLE("apple"),
        ORANGE("orange");

    // Order of initialisation might need adjusting, I haven't tested it.
    private static final Map<String, Fruit> lookup = new HashMap<String, Fruit>();
    private final String fruitname;
    Fruit(String fruitname) {
        this.fruitname = fruitname;
        lookup.put(fruitname, Fruit);
    }
    public String fruitname() {return fruitname;}

    public static Fruit fromFruitname(String fruitname) {
        return lookup.get(fruitname);
    }
}

But:

  • For small enums it's probably more efficient to step through the list.

Incidentally:

  • In this situation I would have gone with convention and used name() since it's the same as the custom name except for the case (easily fixed.)
  • This solution is more useful when what you have to look up is completely different to the name() value.
Trejkaz
I took permission to fix the example, having the map as static.
KLE
The order of initialization is ok, don't worry.
KLE
Yeah, the lack of static was a typo.
Trejkaz
I can't get this example to compile as I get "Cannot refer to the static enum field <myclass>.Fruit.lookup within an initializer". If I make the map non-static, then the callout fromFruitname complains with "Cannot make a static reference to the non-static field lookup". What am I missing?
davek
The order of initialisation was wrong. Building the map would need to be moved to the from* method it seems, which is odd because most of my custom typesafe enums did do it from the constructor. There must be something different about enum vs. rolling your own enumeration classes.
Trejkaz
+2  A: 

I agree with your desire to have no exception created. It's good for performance (as an exception is worth a thousand instructions, for building the stack trace), and it's logical when you say that it is often the case that it is not found (therefore, it is not an exceptional condition).


I think the for loop you mention is correct, if you have only a few enum values. It will probably have the best performance of all. But I understand you don't want it.


You could build a Map to find your enum values, that would avoid the exception and return the appropriate enum at the same time.

Update : Trejkaz already posted the code that does this.


Also note that sometimes, instead of returning null as the return type when no instance matches, some enum have a dedicated instance for that (call it EMPTY or NOT_FOUND for example). The advantage is that all calling code doesn't have to deal with nulls, and risk no NullPointerException. If needed, there can a boolean method that says isFound()(returns true except for that instance). And codes that would really need to differenciate that values from others still can, while the ones that don't care just pass the instance around without knowledge of this special case.

KLE
Touche. Good call with the "not an exceptional condition." I was thinking that an exception should be treated as such. If it isn't really an exception, it shouldn't be an exception. +1, @KLE.
Rap
+3  A: 

I'll be the contrarian here ... I think your first impulse (to throw an exception) is the right thing to do.

If you're checking within the business logic rather than the UI, there won't be any feedback at that level to the user. (If you're not checking in the UI, we have other problems). Therefore, the proper way to handle it is by throwing an exception.

Of course that doesn't mean you have to have the exception bubble up to the UI level thus short-circuiting the rest of your logic. What I usually do it put the enum assignment in its own little try-catch and handle the exception by reassigning or whatever other elegant solution you've devised.

In short ... you were on the money with your first thought. Go with it. Just change your exception handling a little different.

Rap
+4  A: 

I really don't know a built-in solution. So you may have to write it yourself as a static method.

public enum Fruit {
   ...
   static public boolean isMember(String aName) {
       Fruit[] aFruits = Fruit.values();
       for (Fruit aFruit : aFruits)
           if (aFruit.fruitname.equals(aName))
               return true;
       return false;
   }
   ...
}
NawaMan
"values()" creates a cloned array each time, so it is best not to call it too often. Call it only once and cache the results, or use "EnumSet.allOf(Fruit.class)".
dogbane
A: 

Perhaps you shouldn't be using an Enum at all? If you're regularly having to deal with values that aren't defined in your Enum, perhaps you should be using something like a HashMap<String,Fruit> You can then use containsKey(), to find out if a particular key exist.

Tom
+1  A: 

Just to mention another possibility that'll let your calling code not have to worry about exceptions or conditional checks is to always return a Fruit. If the string is not found, return Fruit.UNKNOWN, for example.

Example:

public enum Fruit {
   public Fruit getValueOf(String name) {
        for (Fruit fruit : Fruit.values()) {
           if (fruit.fruitname.equals(name))
               return fruit;
           }
        }
        return UNKNOWN;
   }
   ...
}
JRL
A: 

You can use this static method:

public static <T extends Enum<T>> boolean isMember(Class<T> enumType, String name) {
    for (Enum<T> constant : enumType.getEnumConstants()) {
        if (constant.toString().equals(name)) {
            return true;
        }
    }
    return false;
}
Vitalii Fedorenko
A: 

This is how you can do it using EnumSet.allOf to populate a map:

public enum Fruit {

    APPLE("apple"), 
    ORANGE("orange");

    private static final Map<String, Fruit> nameToValueMap = new HashMap<String, Fruit>();

    static {
        for (Fruit value : EnumSet.allOf(Fruit.class)) {
            nameToValueMap.put(value.name(), value);
        }
    }

    private final String fruitname;

    Fruit(String fruitname) {
        this.fruitname = fruitname;
    }

    public String fruitname() {
        return fruitname;
    }

    public static Fruit forName(String name) {
        return nameToValueMap.get(name);
    }
}
dogbane