2

I want to know what is the best way to handle a NumberFormatException?

A NumberFormatException gets thrown in my code whenever a user passes an empty string. There are multiple of ways to handle this but I am not sure what the best approach is?

Here is my code as is now:

if(result != null) {
    String [] values = result.trim().split(",");
    // This is where the NumberFormatException occurs
    long docId = Long.parseLong(values[0]);
    //There is more to the code but the rest is unnecessary :)  
    break;
}

Should I just avoid the NumberFormatException all together by checking if values[0] is empty before parsing? So something like this:

if(result!= null) {
    String [] values = result.trim().split(",");        
    if("".equals(values[0]) {
        // This method returns void hence the empty return statement.
        return;
    } 
    long docId = Long.parseLong(values[0]);
    break;
}

Or should I catch the exception if it occurs?

if(result!= null) {
    String [] values = result.trim().split(",");        
    try{            
        long docId = Long.parseLong(values[0]);
    } catch (NumberFormatExeption e) {
        LOGGER.warn("Exception in ThisClass :: ", e);
        return;
    }
    break;
}

Or is there another approach you recommend?

robben
  • 181
  • 3
  • 13
  • Perhaps this is similar: http://stackoverflow.com/questions/4410107/what-is-the-proper-way-to-handle-a-numberformatexception-when-it-is-expected – Bijay Gurung Dec 29 '16 at 01:20
  • This is NOT a duplicate! I am asking for the best practice. Please reopen the question @TimBiegeleisen. – robben Dec 29 '16 at 01:27
  • Agreed. This question is not a duplicate of that one. – Stephen C Dec 29 '16 at 01:59
  • May be use [NumberUtils](https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/math/NumberUtils.html) – Ambrish Dec 29 '16 at 02:31

4 Answers4

3

The simplest approach is to catch the NumberFormatException.

Your alternative has a couple of problems:

  1. It is possible that you will get something that is neither a number or an empty string. It is also possible to get an integer that is too large for an int. If these happen, then your fix doesn't work.

  2. A return in the middle of a switch statement is liable to make your code harder to understand. Depending on the context, this could be a problem.

If you decide to go down the path of avoiding all possible cases of NumberFormatException, then you need to check for all possible parseable integers. This can be done using a regex ... modulo that checking for integers that are too large for int (or long) in a regex is really messy.


In this case, my recommendation is to let the exceptions happen, and catch / handle them. Unless .....

  • If empty strings are a common case, then it would be more efficient to deal with them as a special case. Ditto for other unparseable input. Whether the efficiency matters enough to warrant the extra complexity is context dependent.

  • If other "bad number" cases are entirely unanticipated, it may be appropriate to NOT handle NumberFormatException, allow the exception to abort the request / crash the application, and then deal with tracking down the source of the bad data as a maintenance issue. (This also depends on the context ... obviously.)


Best practice on handling NumberFormatExeption

In this context, "best practice" is an oxymoron.

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216
  • Thanks! The reason I suggested to not catch the NFE is because I read that catching runtime exceptions isn't desired i.e. you should avoid it from happening. – robben Dec 29 '16 at 02:26
  • ALso, what do you mean by "it is a problem in returning in the catch block"? And yea, we noticed a very high amount of empty strings which was causing the NFE – robben Dec 29 '16 at 02:29
  • *"I read that catching runtime exceptions"* - That sounds wrong. Certainly it is wrong as a general statement. Catching and recovering from `RuntimeException` is wrong though ... because you have no way of knowing what exception you will catch, what caused it, and what to do about it. – Stephen C Dec 29 '16 at 02:36
  • *"what do you mean by "it is a problem in returning in the catch block"?"* - It makes your code harder to read if you have `return` statements happening at "random" places. – Stephen C Dec 29 '16 at 02:38
1

The NFE can be thrown for various reasons, not just a user providing an empty string. The second approach handles those. The first approach can still throw a NFE!

Also, IMHO, this is simple data validation of user supplied data. So you probably want to give a proper error message to the user. (a WARN log is probably not sufficient)

In that case, an exception could be an appropriate way to handle it, i.e. either wrap the NFE in something nicer like a ValidationException of sorts. (you probably will have other validity checks down the road: parsed long being in a certain range etc.)

And I don't think it is necessary to try to avoid getting an NFE by using regular expressions before parsing the long. Not worth the effort.

Jochen Bedersdorfer
  • 4,093
  • 24
  • 26
  • Thanks! The reason I suggested to not catch the NFE is because I read that catching runtime exceptions isn't desired i.e. you should avoid it from happening. – robben Dec 29 '16 at 02:27
  • That depends on the exception, really. Yes, many RuntimeExceptions are an indicator that there is a bug in the program (like the famous NullPointerException), but in the case of NFE, the designers probably thought it would be too much of a burden to make it a checked exception. – Jochen Bedersdorfer Dec 29 '16 at 02:29
1

If your application parses numbers more than occasionally, you should create a class to standardize parsing -- and potentially also defaulting -- behavior.

There are two general approaches to parsing invalid data:

  1. Fail-fast -- allow the exception to be thrown & propagated, with a meaningful diagnostic message. Business logic will be terminated.
  2. Tolerant -- use a default value when erroneous input is provided, preferably logging a warning. Business logic will operate with the default value. (Your question implies you are leaning to this approach.)

The absence of data (null/empty/blank string) can be treated in one of three ways:

  1. empty means the absence of a value (less common, results in null),
  2. empty means the default value, (most common)
  3. empty is an error.

Long story short, most common approach is to provide a parseInt (String s, int defaultVal) function which is tolerant and answers both blanks & errors as the default value.

public static int parseInt (String s, int defaultVal) {
    if (StringUtils.isBlank(s))
        return defaultVal;

    try {
        return Integer.parseInt(s);
    } catch (NumberFormatException x) {
        log.warn("unparseable integer value: "+s+", using default", x);
        return defaultVal;
    }
}

This can be placed in a parsing utility class, or other suitable location.

One last note: parsing used to be used for both configuration & input data, but dependency injection frameworks such as Spring and CDI can now perform most configuration parsing for you. Parsing remains relevant for input data, however.

Thomas W
  • 13,940
  • 4
  • 58
  • 76
0

First approach can't handle various cases hence I prefer to use second approach.

Shinji Yamada
  • 376
  • 2
  • 7