patternjavaMinor
DB-to-Java value mapper
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
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
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
To help with code-maintenance, I would refactor the comparisions into seperate stateless comparator classes, each will implement one function, the
The list of comparators will be initialized at some early point in the lifecycle of your application and put in a list. The method
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.
This is the interface mentioned.
This is taking the float
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-elsepublic 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.