5
\$\begingroup\$

I am trying to write a method that takes in a String and returns an Object. The catch is I want to parse it to one of a few different data-types, and if that fails, return the original String. I currently have the code below. Is this the best way to handle what I want or is there something obvious that I'm missing?

private static Object tryParse(String str) {
    try {
        return Long.parseLong(str);
    } catch (NumberFormatException ex1) {
        try {
            return Double.parseDouble(str);
        } catch (NumberFormatException ex2) {
            return str;
        }
    }
}

EDIT: I made a tweak that I think should improve performance a bit since I only need to check for longs, doubles, and Strings.

private static Object tryParse(String str) {
    try {
        Double d = Double.parseDouble(str);
        if (d % 1 == 0) {
            return d.longValue();
        }
        return d;
    } catch (NumberFormatException ex1) {
        return str;
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Your second implementation will return a wrong value for "1e30", which is an integer number, but too large for long. \$\endgroup\$ – Roland Illig Jun 13 at 19:20
3
\$\begingroup\$

Your second code, while might be slightly more efficient than the first due to the avoidance of the second try ... catch inside the first, has two bugs:

First, if str contains a value greater than \$2^{63}-1\$ or less than \$-2^{63}\$, while it is an integer value (d % 1 == 0 will be true), it cannot be expressed as a long, so d.longValue() will discard the upper bits.

Second, if str contains a value which can be parsed as a Long, but has a magnitude greater than \$2^{52}\$, parsing it as a Double will discard some of the low-order bits.

jshell> Double d = Double.parseDouble("10000000000000123");
d ==> 1.0000000000000124E16

jshell> d.longValue()
$2 ==> 10000000000000124

jshell> Long.parseLong("10000000000000123");
$3 ==> 10000000000000123

The statements return Long.parseLong(str) and return Double.parseDouble(str) may be doing more work than necessary. These methods return a long and a double, respectively, which then need to be auto-boxed into Long and Double. You should instead use Long.valueOf(str) and Double.valueOf(str) which return already boxed values.


I would write this code like:

private static Object tryParse(String str) {
    try {
        return Long.valueOf(str);
    } catch (NumberFormatException ex) { /* no-op */ }

    try {
        return Double.valueOf(str);
    } catch (NumberFormatException ex) { /* no-op */ }

    return str;
}

... which avoids the nested try-catch blocks, so can easily be extended to several other types, possibly even looping over a collection of parsers:

private static Object tryParse(String str) {
    for (Parser parser : parsers) { 
        try {
            return parser.valueOf(str);
        } catch (NumberFormatException ex) {
            /* no-op */
        }
    }

    return str;
}

or even

private static Object tryParse(String str) {
    for (Parser parser : parsers) { 
        Optional<Object> value = parser.valueOf(str);
        if (!value.empty()) {
            return value.get();
        }
    }

    return str;
}

Implementation of Parser, and its instances, left as exercise to student.

\$\endgroup\$
  • \$\begingroup\$ Oh wow, thanks a ton for the detailed answer. I didn't consider either of those bugs as possibilities, but now that you mention them it does make perfect sense. I'm probably going to go with the first of your three recommended solutions just because my co-workers often complain about my over-engineering of things and that Parser system might get some complaints (even though it's definitely the most elegant system). Thank you so much! \$\endgroup\$ – Michael Ziluck Jun 13 at 20:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.