CODEWITHSUNDEEP
javaenumscorrectness
Antoine Claval
Antoine Claval

Reputation: 4944

is my Enumeration correct?

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.

Upvotes: 5

Views: 505

Answers (4)

Andreas Dolk
Andreas Dolk

Reputation: 114807

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.

Upvotes: 1

newacct
newacct

Reputation: 122489

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

Upvotes: 1

skaffman
skaffman

Reputation: 403581

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.

Upvotes: 5

jjnguy
jjnguy

Reputation: 138932

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.

Upvotes: 1

Related Questions