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.

Reata

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. – Ornithischian

I wonder why do you use Asset.class.getSimpleName() instead of just writing "Asset" ? Do you plan to change the name by refactoring? – Idolize

Because it's always good practice to use class literals instead of strings? – Ornithischian

@skaffman: Okay, then why aren't there an Unknown and Any class? – Idolize

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. – Ornithischian

possible duplicate of Get enum by its inner field – Symbol

Answer 1

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.

Ornithischian

I just spent 10 minutes or so shouting at the compiler – Ornithischian

If there are are 100's of elements in the enum (or even <ugh> thousands) then this might be a performance improvement ... might, especially if you are looking these up a lot. – Alemanni

And store it where? EnumMap is just an optimized Map, it's not magic. – Ornithischian

Sorry, I had in my head Enum->AlternateNameString, which is backwards from what is desired. – Plutonic

Answer 2

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.

Valorous

Answer 3

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.

Theoretician

Answer 4

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;
}

Indicant

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. – Reata