How to avoid excessive code duplication when using enums in Java

I am refactoring some legacy code and have come across a problem which I'm sure has a elegant solution - but I can't quite get there.

Initially there were a load of classes which extended an abstract class BaseType. Each of these classes has a enum - XmlElementTag - with values specific to the class:

enum XmlElementTag {value1, value2, value3}

They each also have a method :

private XmlElementTag getTag(String s){
    XmlElementTag ret = null;
    try {
        ret = XmlElementTag.valueOf(s);
    } catch (Exception e) {
        Log.e(this, s+" is not supported tag");
    }
    return ret;
}

Every class has this exact same getTag method, but obviously they are all referring to the XmlElementTag enum specific to the class they are in. So, I'd like to get rid of this code duplication if I can.

I thought that maybe I could use a marker interface to solve this problem, so created one as which each XmlElementTag enum now inherits and rewrote the getTag method and put it in the super class.

So I have this in each class:

private XmlElementTag implements GenericTag {value1, value2, value3}; 

And this in the BaseType superclass:

public interface GenericTag {}

protected GenericTag 开发者_如何学运维getTag(String tagName){
    XmlElementTag tag = null;
    try {
        tag = XmlElementTag.valueOf(tagName);
    } catch (Exception e) {
        Log.e(this, tagName+" is not supported tag");
    }
    return tag;
}

But again this doesn't work as the BaseType super class doesn't know what XmlElementTag is; Java doesn't allow abstract class variables; and creating this element in the BaseType won't work, as the getTag code will always refer to this enum, rather than the one in the class which extends BaseType.

Can anyone point me in the correct direction?

I guess you could write a static generic helper method that did what getTag does. It would need to use reflection under the hood, and would most likely require you to pass the enumeration's Class object as a parameter.

But IMO, you shouldn't. The getTag() method is kind of wrong-headed. It is turning what is effectively bad input into a null. That's wrong from two perspectives:

• In most contexts, "you gave me bad stuff" should not be treated as "you gave me nothing".
• If you are not scrupulously careful, those null values are going to come back to bite you as NullPointerExceptions.

So really, your application code should either catch and deal with the IllegalArgumentException that arises when the conversion goes wrong, or it should allow the exception to bubble up to the top where it can be reported as (for instance) an error parsing the input stream.

(I don't think that an enum can either extend or be extended, so I don't think your enums can inherit a generic version of this class.)

You might be able to coalesce the XmlElementTag elements into a single enum and establish an EnumSet apropos to each derived type. There's an example here.

Addendum: In this scheme, getTag() would then become a single method of the combined enum. Each derived class would invoke getTag() using the Set that it considers valid. The method might have a signature such as this:

public static XmlElementTag getTag(Set valid, String s) { ... }

Unfortunately Java enums don't come with a good meta-class (Class is evil). However, all you really need here is the list (array) of the enum values.

As it's a private method, you might as well use composition.

import static java.util.Objects.requireNonNull;

/* pp */ class EnumFinder<E extends Enum<E>> {
    private final E[] tags;
    protected BaseType(E[] tags) {
        this.tags = requireNonNull(tags);
    }

    public E getTag(String name) {
        requireNonNull(name);
        for (E tag : tags) {
            if (name.equals(tag.name())) {
                return tag;
            }
        }
        Log.e(this, name+" is not supported tag"); // (sic)
        return null; // (sic)
    }
    ...
}

public class DerivedType {
    private static final EnumFinder<XmlElementType> finder = // note, shared
        new EnumFinder<>(XmlElementType.values());
    ...
        finder.getTag(name)
    ...
}

(Create a Map<String,E> if you really want to. Unnecessary for reasonably sized enums.)

If you really want to use inheritance, then that is much the same. (Unfortunately as we are using an array, unless you add more boilerplate to your code, this code will create an unnecessary extra array per instance - probably not a significant issue, but may be.):

/* pp */ abstract class BaseType<E extends Enum<E>> {
    private final E[] tags;
    protected BaseType(E[] tags) {
        this.tags = requireNonNull(tags);
    }

    public E getTag(String name) {
        requireNonNull(name);
        for (E tag : tags) {
            if (name.equals(tag.name())) {
                return tag;
            }
        }
        Log.e(this, name+" is not supported tag"); // (sic)
        return null; // (sic)
    }
    ...
}
public class DerivedType extends BaseType<XmlElementType> {
    public DerivedType() {
        super(XmlElementType.values());
    }
    ...
        this.getTag(name)
    ...
}

You could use generics for that:

The Base is

public abstract class Base {
protected static <T extends Enum<T>> T getTag(Class<T> enumType, String s) {
    T ret = null;
    try {
        ret = Enum.valueOf(enumType, s);
    } catch (Exception e) {
        System.err.println(s + " is not supported tag");
    }
    return ret;
}

protected abstract <T extends Enum<T>> T getTag(String s);
}

Your numerous classes have a shorter getTag() (and all the logic is in the Base)

public class ClassA extends Base {
enum XmlElementTag {
    UL, LI
}

@Override
protected XmlElementTag getTag(String s) {
    return Base.getTag(XmlElementTag.class, s);
}
}

(same thing for ClassB)

I think that you wanted to achieve something like this (correct me if I'm wrong):

interface GenericTag {

    public GenericTag fromString(String str) throws IllegalArgumentException;
}


class BaseType {

    protected GenericTag getTag(String tagName) {
        GenericTag tag = null;
        try {
            tag = tag.fromString(tagName); 
        } catch (Exception e) {
            Log.e(this, tagName+" tag is not supported");
        }
        return tag;
    }

}

class ConcreteTypeA extends BaseType {

    enum XmlElementTag implements GenericTag {
        TAG1, TAG2;

        public GenericTag fromString(String str) throws IllegalArgumentException {
            return XmlElementTag.valueOf(str);
        }
    }
}

However, that will never work. You would need the fromString method to return an instance of appropriate class (in this case, enum) implementing the GenericTag, but the fromString method has to be performed by that concrete class, which you don't have yet, so you'll get a NullPointerException. It's a sort of Chicken and Egg Problem! :)