Data type detection for a CSV file parser I am building a CSV file parser, and in order to get the appropriate Object to represent different data types found on the parsed files, I wrote the following function: 

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);
    }else{
        String integerPattern = detectInteger(valueAsString);
        if(integerPattern != null){
            return stringToBigInteger(valueAsString);
        }else{
            String datePattern = detectDate(valueAsString);
            if(datePattern != null){
                return stringToDate(valueAsString, datePattern);
            }else{
                // value is a String... nothing else to do!
                return valueAsString;                   
            }
        }
    }
}


In order to call the appropriate 'casting' methods (stringToDate, stringToBigInteger, stringToBigDecimal), I need to use the specific pattern string that gets returned when detecting if a particular value is of that type.
Hence, the pattern value gets used for routing to the appropriate function, and also as an argument in that function call.

To make this method logic 'flatter', I could avoid passing the pattern as an argument, and calling the pattern detection method (detectDecimal, detectInteger, detectDate) again from within the 'casting' method, but that would lead to parsing the same value two times, which seems wasteful at best. 

So, is this the best way this method can be written without getting to call the detection methods twice? 

EDIT

Here are the detectDate and stringToDate methods, as requested (the methods for BigInteger and BigDecimal are very similar). Please let me know if I need to post anything else:

private static String detectDate(String dateString) {
    for (String regexp: DATE_FORMAT_REGEXPS.keySet()) {
        if(dateString.toLowerCase().matches(regexp)) {
            return DATE_FORMAT_REGEXPS.get(regexp);
        }
    }
    return null; // value is not a date
}

public static Date stringToDate(String valueAsString, String format) throws ParseException{
    SimpleDateFormat dateFormat = new SimpleDateFormat(format);
    Date date = dateFormat.parse(valueAsString);
    return date;
}
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&lt;String, String&gt; 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&lt;Function&lt;String, Object&gt;&gt; FUNCTIONS = 
    Arrays.asList(s -&gt; tryDecimal(s), s -&gt; tryInteger(s), s -&gt; 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 -&gt; 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&lt;String&gt; patternDetector;
    private final BiFunction&lt;String, String, Object&gt; toObjectFunction;

    public Conversion(UnaryOperator&lt;String&gt; patternDetector, BiFunction&lt;String, String, Object&gt; toObjectFunction) {
        this.patternDetector = patternDetector;
        this.toObjectFunction = toObjectFunction;
    }

    public UnaryOperator&lt;String&gt; getPatternDetector() {
        return patternDetector;
    }

    public BiFunction&lt;String, String, Object&gt; 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&lt;Conversion&gt; CONVERSIONS = Arrays.asList(
    new Conversion(v -&gt; detectDecimal(v), (v, p) -&gt; stringToBigDecimal(v, p)),
    new Conversion(v -&gt; detectInteger(v), (v, p) -&gt; stringToBigInteger(v)),
    new Conversion(v -&gt; detectDate(v), (v, p) -&gt; stringToDate(v, p))
);

public static Object stringToDataType(String valueAsString) throws ParseException {
    return CONVERSIONS.stream()
                      .map(c -&gt; 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.