views:

126

answers:

1

Hi, I am refactoring a part of our legacy app which handles exporting and importing of DB tables from/to Excel sheets. We have a Formatter subclass for each table, to provide the definition of that table: how many columns it has, and what is the name, format and validator of each column. The getters which supply this data are then called by a Template Method which exports/imports the table. I have extracted the column data into an enum, which greatly simplified the code. A formatter now looks like this (some details omitted for brevity):

public class DamageChargeFormatter extends BaseFormatter {
    public static final int NUM_COLUMNS = 7;

    public enum Column {
        VEHICLE_GROUP(0, "Vehicle Group", /* more params */),
        NAME_OF_PART(1, "Name of Part", /* more params */),
        //...
        LOSS_OF_USE(6, "Loss of Use", /* more params */);

        private static final Map<Integer, Column> intToColumn = new HashMap<Integer, Column>();

        static {
            for (Column type : values()) {
                intToColumn.put(type.getIndex(), type);
            }
        }

        public static TableColumn valueOf(int index) {
            return intToColumn.get(index);
        }

        private int index;
        private String name;

        Column(int index, String name, /* more params */) {
            this.index = index;
            this.name = name;
            //...
        }

        public int getIndex() { return index; }

        public String getName() { return name; }

        // more members and getters...
    }

    protected String getSheetName() {
        return "Damage Charges";
    }

    public String getColumnName(int columnNumber) {
        TableColumn column = Column.valueOf(columnNumber);

        if (column != null) {
            return column.getName();
        }
        return null;
    }

    // more getters...

    protected int getNumColumns() {
        return NUM_COLUMNS;
    }

    protected boolean isVariableColumnCount() {
        return false;
    }
}

Now, I have about a dozen such classes, each of which containing exactly the same code except that NUM_COLUMNS and the enum values of Column are different. Is there any way to genericize this somehow? The main obstacle to this is the static Column.valueOf() method and the static constant NUM_COLUMNS. Another concern with latter is that it really belongs to an abstraction one level higher, i.e. to the table, not to an individual column - it would be nice to somehow incorporate this into the generic solution.

Technically I could solve this with a base interface (TableColumn below) and reflection, but I don't really like that, as apart from trading compile time errors to runtime errors, it makes the code ugly (to me):

public class GenericFormatter<E extends TableColumn> extends BaseFormatter {
    private Method valueOfMethod;

    public GenericFormatter(Class<E> columnClass) {
        try {
            valueOfMethod = columnClass.getDeclaredMethod("valueOf", Integer.class);
        } catch (NoSuchMethodException e) {
            throw new RuntimeException(e);
        }
    }

    public String getColumnName(int columnNumber) {
        try {
            @SuppressWarnings("unchecked")
            E elem = (E) valueOfMethod.invoke(columnNumber);

            if (elem != null) {
                return elem.getName();
            }
        } catch (Exception e) {
            throw new RuntimeException(e);
        }
        return null;
    }

    //...
}

Note that this code is purely experimental, as yet untested...

Is there a nicer, cleaner, safer way?

+1  A: 

May be, something like this:

public class TableMetadata<E extends Enum & TableColumn> {
    private Map<Integer, TableColumn> columns = new HashMap<Integer, TableColumn>();

    public TableMetadata(Class<E> c) {
        for (E e: c.getEnumConstants()) {
            columns.put(e.getIndex(), e);
        }
    }

    public String getColumnName(int index) {
        return columns.get(index).getName();
    }
}

public class GenericFormatter<E extends TableColumn> extends BaseFormatter {  
    private TableMetadata<E> m;  

    public GenericFormatter(TableMetadata<E> m) {  
         this.m = m;
    }  

    public String getColumnName(int columnNumber) {  
        return m.getColumnName(index);
    }  

    //...  
}

EDIT: Enum added to the type parameter for more compile-time safety

axtavt
Excellent idea! You made it obvious that I was stuck in a mental rut, blindly assuming that the static lookup method is inside the enum. Thanks! :-)
Péter Török