HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

DB-to-Java value mapper

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
mappervaluejava

Problem

In my company, I've inherited some Java library that I'm now writing tests to, refactoring and fixing Sonar issues.

One particular point that Sonar is complaining about is a big chaining of if/else if/else statements, that is used in a class responsible for parsing values that come from the DB to the corresponding values used in Java. The cyclomatic complexity of this method is 32, where Sonar is configured to a threshold of 15.

I would like some ideas of a better way to write this. Maybe some design-pattern that could be applied.

Since this is a pretty critical part of the application, it would be interesting not to add a considerable overhead.

```
private Object getValue(final Class type, final int index, final ResultSet resultSet) throws SQLException {
Object value = null;

if (Boolean.class.isAssignableFrom(type) || boolean.class.isAssignableFrom(type)) {
String v = resultSet.getString(index);
if (v != null) {
value = "Y".equalsIgnoreCase(v);
}
} else if (Byte.class.isAssignableFrom(type) || byte.class.isAssignableFrom(type)) {
value = resultSet.getByte(index);
} else if (Byte[].class.isAssignableFrom(type) || byte[].class.isAssignableFrom(type)) {
value = resultSet.getBytes(index);
} else if (Short.class.isAssignableFrom(type) || short.class.isAssignableFrom(type)) {
value = resultSet.getShort(index);
} else if (Integer.class.isAssignableFrom(type) || int.class.isAssignableFrom(type)) {
value = resultSet.getInt(index);
} else if (Long.class.isAssignableFrom(type) || long.class.isAssignableFrom(type)) {
value = resultSet.getLong(index);
} else if (Float.class.isAssignableFrom(type) || float.class.isAssignableFrom(type)) {
value = resultSet.getFloat(index);
} else if (Double.class.isAssignableFrom(type) || double.class.isAssignableFrom(type)) {
value = resultSet.getDouble(index);
} else if (BigDecimal.class.isAssignableFrom

Solution

I think, performance wise, you won't get much better than the if-then-else construct. Since isAssignableFrom is not a direct lookup, but a rather complex task, but your runtime types are actually limited, it may be helpful to build a cache around the result. So, if you found a type match, keep that in a direct lookup cache to avoid walking the if-then.

To help with code-maintenance, I would refactor the comparisions into seperate stateless comparator classes, each will implement one function, the getValue method.

The list of comparators will be initialized at some early point in the lifecycle of your application and put in a list. The method getValue will be reduced to a for loop over the comparator-list.

Adding another type is now reduced to changing a comparator class. This makes it very obvious, what you have changed even in changelogs, since the class is mentioned in the committed-files-list.

To help maintenance even more, you could introduce either a search of your classpath for the comparator interface or introduce an annotation to mark the implementations for automatic building of the list.

This will give you an almost self-maintaining code for the given task. Whenever you need a new type, you just add it with the right interface/annotation and it will magically be seen on the next run.

This would be your main entry point.

public class ValueGetter {

    private static List converter;

    static {
        // might require synchronize
        converter = new ArrayList<>(1);
        converter.add(new FloatConverter());
        // ... add more or build using reflection or whatever
    }

    public Object getValue(final Class type, final int index, final ResultSet resultSet) throws SQLException {
        Object value;

        for (TypeConverter c : converter) {
            value = c.getValue(type, resultSet, index);
            if (value != TypeConverter.NOTTYPE) {
                return value;
            }
        }

        throw new IllegalArgumentException("Cannot convert to requested type");
    }
}


This is the interface mentioned.

public interface TypeConverter {

    public static final String NOTTYPE = "not of this type";

    Object getValue(Class type, ResultSet resultSet, int index) throws SQLException;
}


This is taking the float if-else

public class FloatConverter implements TypeConverter {
    @Override
    public Object getValue(Class type, ResultSet resultSet, int index) throws SQLException {
        if (Float.class.isAssignableFrom(type) || float.class.isAssignableFrom(type)) {
            return resultSet.getFloat(index);
        }

        return NOTTYPE;
    }
}

Code Snippets

public class ValueGetter {

    private static List<TypeConverter> converter;

    static {
        // might require synchronize
        converter = new ArrayList<>(1);
        converter.add(new FloatConverter());
        // ... add more or build using reflection or whatever
    }

    public Object getValue(final Class<?> type, final int index, final ResultSet resultSet) throws SQLException {
        Object value;

        for (TypeConverter c : converter) {
            value = c.getValue(type, resultSet, index);
            if (value != TypeConverter.NOTTYPE) {
                return value;
            }
        }

        throw new IllegalArgumentException("Cannot convert to requested type");
    }
}
public interface TypeConverter {

    public static final String NOTTYPE = "not of this type";

    Object getValue(Class<?> type, ResultSet resultSet, int index) throws SQLException;
}
public class FloatConverter implements TypeConverter {
    @Override
    public Object getValue(Class<?> type, ResultSet resultSet, int index) throws SQLException {
        if (Float.class.isAssignableFrom(type) || float.class.isAssignableFrom(type)) {
            return resultSet.getFloat(index);
        }

        return NOTTYPE;
    }
}

Context

StackExchange Code Review Q#132136, answer score: 3

Revisions (0)

No revisions yet.