is my Enumeration correct?

Question

All over our project, we have this kind of enums. They works just fine, but we are not sure about them.

Specially with the getDocumentType(String) method.

Is there a way to avoid the iteration over all the Enums field ?

public enum DocumentType {

    UNKNOWN("Unknown"),
    ANY("Any"),
    ASSET(Asset.class.getSimpleName()),
    MEDIA(Media.class.getSimpleName()),
    MEDIA35MM(Media.class.getSimpleName() + " 35mm");


    private String label;

    private DocumentType(String label) {
        this.label = label;
    }

    public String getLabel() {
        return label;
    }

    public static DocumentType getDocumentType(String label){
        for(DocumentType documentType : DocumentType.values()){
            if(documentType.getLabel().equals(label)){
                return documentType;
            }
        }
        return UNKNOWN;
    }
}

Edit : Check the newacct response. She's fine too.

It's tempting to store a static Map of labels to enum instances, but frustratingly, java won't let you reference a static field from an enum's constructor. — skaffman, Jul 29 '09 at 12:31
I wonder why do you use Asset.class.getSimpleName() instead of just writing "Asset" ? Do you plan to change the name by refactoring? — akarnokd, Jul 29 '09 at 12:43
Because it's always good practice to use class literals instead of strings? — skaffman, Jul 29 '09 at 12:45
@skaffman: Okay, then why aren't there an Unknown and Any class? — akarnokd, Jul 29 '09 at 12:46
Because you don't invent a class just so you can refer to its name, but if the class already exists, then use the literal. — skaffman, Jul 29 '09 at 14:55
possible duplicate of [Get enum by its inner field](http://stackoverflow.com/questions/2780129/get-enum-by-its-inner-field) — polygenelubricants, May 27 '10 at 17:36

skaffman · Accepted Answer · 2009-07-29T12:43:44.980

5

You're going to have to do that iteration somewhere, due to the restrictions in writing enums. In an ideal world, you would populate a static Map from within DocumentType's constructor, but that's not allowed.

The best I can suggest is performing the iteration once in a static initializer, and storing the enums in a lookup table:

public enum DocumentType {

    .... existing enum stuff here

    private static final Map<String, DocumentType> typesByLabel = new HashMap<String, DocumentType>();
    static {
        for(DocumentType documentType : DocumentType.values()){
            typesByLabel.put(documentType.label, documentType);
        }
    }

    public static DocumentType getDocumentType(String label){
        if (typesByLabel.containsKey(label)) {
            return typesByLabel.get(label);
        } else {
            return UNKNOWN;
        }
    }
}

At least you won't be doing the iteration every time, although I doubt you'll see any meaningful performance improvement.

edited Jul 29 '09 at 12:43

answered Jul 29 '09 at 12:38

skaffman

398,947
96
818
769

1

I just spent 10 minutes or so shouting at the compiler – skaffman Jul 29 '09 at 12:40
If there are are 100's of elements in the enum (or even thousands) then this might be a performance improvement ... might, especially if you are looking these up a lot. – aperkins Jul 29 '09 at 13:29
1

And store it where? EnumMap is just an optimized Map, it's not magic. – skaffman Jul 29 '09 at 20:26
Sorry, I had in my head Enum->AlternateNameString, which is backwards from what is desired. – Chris Kessel Jul 30 '09 at 14:21

score 1 · Answer 2 · answered Jul 29 '09 at 12:34

As far as I know (For what it's worth), that is the best way to do what you want.

That is how I would do it at least.

If your enum count grows significantly (couple hundred - thousands) you may want to add a Maping of Strings to enums to do the look-up a little faster. But for the small amount of eunums you have, this may be overkill.

score 1 · Answer 3 · edited May 27 '10 at 17:32

1

Looks fine to me.

I'd leave the iteration as it is. Sure you could add a Map<'label','DocumentType'> implementation to the enum class and do a lookup but it won't increase performance significantly.

edited May 27 '10 at 17:32

jball

24,791
9
70
92

answered Jul 29 '09 at 12:34

Andreas Dolk

113,398
19
180
268

newacct · Answer 4 · 2009-07-30T01:39:42.250

1

If the strings are known at compile-time, and if they are valid identifiers, you can just use them as the names of the enums directly:

public enum DocumentType { Unknown, Any, Asset, Media, Media35mm }

and then get it by .valueOf(). For example:

String label = "Asset";
DocumentType doctype;
try {
    doctype = DocumentType.valueOf(label);
} catch (IllegalArgumentException e) {
    doctype = DocumentType.Unknown;
}

edited Jul 30 '09 at 01:39

answered Jul 29 '09 at 18:46

newacct

119,665
29
163
224

A colleague was comming with the same solution. It's fine for most of the time, not in our precise case, but for next iteration will will check if we really nead a identifier and a label. – Antoine Claval Jul 30 '09 at 08:05

is my Enumeration correct?

4 Answers4