Yes, you can make that code a lot more readable by noticing a simple fact. When you have the code
String decimalPattern = detectDecimal(valueAsString);
if(decimalPattern != null){
return stringToBigDecimal(valueAsString, decimalPattern);
}else{
// ...
}
The else part is actually unnecessary. If the test is true then we return with a value. This means that you exit early from the method, without going further down.
So you can rewrite it like this:
String decimalPattern = detectDecimal(valueAsString);
if(decimalPattern != null){
return stringToBigDecimal(valueAsString, decimalPattern);
}
// ...
As you found out, this has the advantage that the subsequent code is shifted by one indentation to the left, making it easier to read. So only with this change, the code now looks like:
public static Object stringToDataType(String valueAsString) throws ParseException {
// detections ordered by probability of occurrence in Buffer_Bank.
String decimalPattern = detectDecimal(valueAsString);
if (decimalPattern != null) {
return stringToBigDecimal(valueAsString, decimalPattern);
}
String integerPattern = detectInteger(valueAsString);
if (integerPattern != null) {
return stringToBigInteger(valueAsString);
}
String datePattern = detectDate(valueAsString);
if (datePattern != null) {
return stringToDate(valueAsString, datePattern);
}
return valueAsString;
}
You posted an example of your detect* and stringTo* methods. In fact, you don't need two methods. You only need a single one that is specific to the current type you want to test. What you want is:
- Try to interpret the value as a decimal and return it;
- If not, try to interpret the value as an integer and return it;
- If not, try to interpret the value as a date and return it.
Said like this, it is possibe to realize that you only need 3 methods: tryDate, tryInteger and tryDecimal. Each of those will do the necessary to try to interpret the String and return null if they can't. Basically, the code you currently have inside stringTo* should be merged into the part where you found the correct pattern in detect*. As an example:
private static Object tryDate(String dateString) {
for (Map.Entry<String, String> entry : DATE_FORMAT_REGEXPS.entrySet()) {
if (dateString.toLowerCase().matches(entry.getKey())) {
SimpleDateFormat dateFormat = new SimpleDateFormat(entry.getValue());
try {
return dateFormat.parse(dateString);
} catch (ParseException e) {
// what to do in this case? Possibilites: throw an exception or return null
}
}
}
return null; // value is not a date
}
Note that I changed your code so that it loops directly around the entries instead of looping just with the keys. You should also decide what to do in case of a ParseException. You could rethrow a custom runtime exception, wrapping the original, or returning null to signal that the attempt failed.
The big issue is that there is some duplicated logic in this: each time we are detecting a pattern and, if it's not null, applying it to the value. If you were to add more detection algorithm, this method can become very quickly copy/pasted code and clumsy. We need to refactor that and make it generic.
Using Java 8, you can define that as a list of Function.
private static final List<Function<String, Object>> FUNCTIONS =
Arrays.asList(s -> tryDecimal(s), s -> tryInteger(s), s -> tryDate(s));
The goal of those functions is to convert a String into a Object. Here, we define 3 which are the 3 methods mentioned above. Then we can have
public static Object stringToDataType(String valueAsString) {
return FUNCTIONS.stream()
.map(f -> f.apply(valueAsString))
.filter(Objects::nonNull)
.findFirst()
.orElse(valueAsString);
}
This creates a Stream pipeline over the functions. Each of them is applied to the given String, non-null elements are filtered and only the first one is kept. Effectively, this will select the first non-null element, so it will select the first mapping that matched. If all are null then we return a default value with orElse which is the given value as String.
If somehow you want to keep your current approach with two different methods (one for detecting the pattern and one for parsins), then what you want comes down to creating a custom class that will hold:
- A function that returns the pattern detected given the value as String;
- A function that returns the object given the value as String and a pattern.
Simply, it could be:
public final class Conversion {
private final UnaryOperator<String> patternDetector;
private final BiFunction<String, String, Object> toObjectFunction;
public Conversion(UnaryOperator<String> patternDetector, BiFunction<String, String, Object> toObjectFunction) {
this.patternDetector = patternDetector;
this.toObjectFunction = toObjectFunction;
}
public UnaryOperator<String> getPatternDetector() {
return patternDetector;
}
public BiFunction<String, String, Object> getToObjectFunction() {
return toObjectFunction;
}
}
Then we can make a static utility that applies a given Conversion to a given value:
private static Object stringToDataType(String valueAsString, Conversion conversion) {
String pattern = conversion.getPatternDetector().apply(valueAsString);
if (pattern != null) {
return conversion.getToObjectFunction().apply(valueAsString, pattern);
}
return null;
}
and finally, we can refactor the main method:
private static final List<Conversion> CONVERSIONS = Arrays.asList(
new Conversion(v -> detectDecimal(v), (v, p) -> stringToBigDecimal(v, p)),
new Conversion(v -> detectInteger(v), (v, p) -> stringToBigInteger(v)),
new Conversion(v -> detectDate(v), (v, p) -> stringToDate(v, p))
);
public static Object stringToDataType(String valueAsString) throws ParseException {
return CONVERSIONS.stream()
.map(c -> stringToDataType(valueAsString, c))
.filter(Objects::nonNull)
.findFirst()
.orElse(valueAsString);
}
Now, if we want to add more conversion in the future, we just need to update this list that, arguably, should be made a constant instead of being initialized each time. The Stream pipeline will return the first non-null element and will short-circuit once it has been found. If all are null, there we return the default value which is the given value as string.
