More German overengineering™ - Class mappings and factories

Goals:

So the plan was simple: Provide a factory to instantiate implementations of a certain interface (ModelConverter), depending on what model-class you want to convert.

The approach is relatively straightforward:

statically map model-classes to ModelConverter implementations

use that mapping to obtain the implementation class
use reflection to get a constructor and invoke it
return the instance


If any of these steps fail, fail hard, because the application relies on the ModelConverter instances.

My code currently looks as follows (note that I am tied to java 6 so please don't recommend multi-catch):

```
public final class ModelConverterFactory {

private enum ClassMapping {
DEFAULT(Object.class, AbstractConverter.class), RIGHT(Right.class,
RightConverter.class);

private Class modelClass;
private Class converterClass;

private ClassMapping(final Class modelClass, final Class clazz) {
if (modelClass == null || clazz == null) {
throw new BrokenCodeException(
"Cannot create a mapping without modelClass and converterClass");
}

this.modelClass = modelClass;
this.converterClass = clazz;
}

public static ClassMapping fromModelClass(final Class modelClass) {
for (ClassMapping mapping : ClassMapping.values()) {
if (mapping.modelClass.equals(modelClass)) {
return mapping;
}
}
return DEFAULT;
}

public Class getConverterClass() {
return this.converterClass;
}

public Class getModelClass() {
return this.modelClass;
}
}

public static Collection> supportedModelClasses() {
Collection> supported = new HashSet>();
for (ClassMapping mapping : ClassMapping.values()) {
if (mapping != ClassMapping.DEFAULT) {
supporte

Things will be greatly simplified if ClassMapping didn't map a class to a class, but a class to an instance.

public final class ModelConverterFactory {

    private enum ClassMapping {
        DEFAULT(Object.class, new AbstractConverter()), RIGHT(Right.class, new RightConverter());

        private Class modelClass;
        private ModelConverter converter;

        private  ClassMapping(final Class modelClass, final ModelConverter modelConverter) {
            this.modelClass = modelClass; // got rid of null checks, we control all invocations, none pass null.
            this.converter = modelConverter;
        }

        public static ClassMapping fromModelClass(final Class modelClass) {
            for (ClassMapping mapping : ClassMapping.values()) {
                if (mapping.modelClass.equals(modelClass)) { 
                    return mapping;
                }
            }
            return DEFAULT;
        }

        public ModelConverter getConverter() {
            return this.converter;
        }

        public Class getModelClass() {
            return this.modelClass;
        }
    }

    public static Collection> supportedModelClasses() {
        Collection> supported = new HashSet>();
        for (ClassMapping mapping : ClassMapping.values()) {
            if (mapping != ClassMapping.DEFAULT) {
                supported.add(mapping.getModelClass());
            }
        }
        return supported;
    }

    public static  ModelConverter getConverterFor(final Class clazz) {
        return (ModelConverter) ClassMapping.fromModelClass(clazz).getConverter(); // safe cast : we know all entries will match
    }
}

This gets rid of that nasty introspection bit. Of course this means ModelConverter implementations ought to be reentrant.

I would also be inclined to replace the enum with a Map. You say you use an enum to disallow runtime changing of mappings, but with proper encapsulation that can be perfectly achieved with a Map as well.

StackExchange Code Review Q#57768, answer score: 6