CODEWITHSUNDEEP
javaparsingexceptionnumberformatexception
user1266094
user1266094

Reputation:

Is catching NumberFormatException a bad practice?

I have to parse a String that can assume hex values or other non-hex values

0xff, 0x31 or A, PC, label, and so on.

I use this code to divide the two cases:

String input = readInput();

try {
    int hex = Integer.decode(input);            
    // use hex ...

} catch (NumberFormatException e) {
   // input is not a hex, continue parsing
}

Can this code be considered "ugly" or difficult to read? Are there other (maybe more elegant) solutions?

EDIT : I want to clarify that (in my case) a wrong input doesn't exist: i just need to distinguish if it is a hex number, or not. And just for completeness, i'm making a simple assebler for DCPU-16.

Upvotes: 10

Views: 2599

Answers (7)

Alex Lockwood
Alex Lockwood

Reputation: 83311

Exception handling is an integral part (and one of the design goals) of the Java programming language... you shouldn't cast them off just because you think they are "ugly".

That said, if you want a simple and readable way to handle NumberFormatExceptions, you might consider using the NumberUtils class instead.

The toInt(String str, int defaultValue) method converts a String to an int, returning a default value if the conversion fails. If the string is null, the default value is returned.

 NumberUtils.toInt(null, 1) = 1
 NumberUtils.toInt("", 1)   = 1
 NumberUtils.toInt("1", 0)  = 1

The method encapsulates exception catching and handling, as seen in the source code below. As a result, the client only needs to make a single method call.

public static int toInt(String str, int defaultValue) {         
    if(str == null) {
        return defaultValue;
    }
    try {
        return Integer.parseInt(str);
    } catch (NumberFormatException nfe) {
        return defaultValue;
    }
}

Upvotes: 7

pb2q
pb2q

Reputation: 59637

In your case I would prefer something like an isHexDigit method to using NumberFormatException, unless there are some assumptions that you can make about the format of your data - from your description it seems that there aren't such assumptions about when you'll encounter hex numbers vs. non-hex numbers.

This is because exceptions should be used to handle exceptional conditions, and if the expectation from your data is: either hex digits or non-hex digits, separated by spaces, then there's nothing exceptional about encountering a token that is other than a hex digit.

Furthermore using the Exception does make the code less readable: without comments about the data, it hides the fact that interspersed non-hex digits are acceptable and expected input.

Having stated that preference, I might use exception handling to handle this case, and I certainly see plenty of code that does so. Lots of good functionality is wrapped up for you in that combination of decode/parseInt/NumberFormatException. I wouldn't use this without an explicit comment that clearly explains what I'm doing.

Upvotes: 1

RalphChapin
RalphChapin

Reputation: 3158

It's the best you can do. A method is either going to return some kind of success/error indicator or its going to throw an exception, it's just a question of which is most convenient. Here Sun's made the decision for us, so no need for a debate.

What does rather bug me about this is that the exception is going to include a full stack trace! In your particular case, if you are reading millions of these strings, you will notice the (completely unnecessary) poor performance. If it matters to you, you might want to consider writing your own method (you can use the Sun code as a guide.) Then you can decide for yourself whether you want to use an exception or not. If you do, keep a static copy of the exception handy and always throw that to save allocation time. And override fillInStackTrace so it does nothing and you don't have a meaningless stack trace in your exception.

Upvotes: 0

Alex Lockwood
Alex Lockwood

Reputation: 83311

No, it's not "bad practice". It just depends on the situation.

For example, as an Android, if the user inputs a string "123a" into a text box that is only supposed to accept integers, and is subsequently parsed, an exception will be thrown causing the application to crash. In this case, it would make perfect sense to catch the exception and prompt the user to re-enter the text.

Upvotes: 1

Kumar Vivek Mitra
Kumar Vivek Mitra

Reputation: 33544

Its Not about how good you code looks, but how good your code works.......... Ya offcourse it should be readable, As famously said...

Any fool can write a code that computer can understand, but only great programmers write codes that humans can understand.

Its perfectly fine in some cases where, you need to have such kind of exceptions in place.

When you want to catch multiple exceptions which belong to the same inheritance tree, then create a try block, and multiple catch blocks from more specific to more abstract.

eg:

`Animal <--- Carnivores <--- Dog`

Now suppose there is a DogException, CarnivoresException, AnimalException.

Then it must be like this,

try{

           // your code 

     }
      catch(DogException d){}
      catch(CarnivoresException c){}
      catch( AnimalException a){}

Above catches has been cascaded from more specific to more abstract so that the exception is caught to it very cause.

If there is no inheritance, then catch can be in any order...

Upvotes: 0

paulsm4
paulsm4

Reputation: 121799

Yours is the second question I've seen today asking about this.

No, it's perfectly appropriate to catch this exception.

And it's definitely better form to catch a more explicit exception (like "NumberFormatException") than a generic "Exception".

IMHO...

PS: Where you put the exception: at this level, or higher-up, is a different question.

The rule of thumb is "the lowest level where you know what happened, and how best to recover."

Or, to put it differently (quoting from a link below):

"A method should only catch an exception when it can handle it in some sensible way."

Here's some discussion:

Upvotes: 2

Rocky Pulley
Rocky Pulley

Reputation: 23321

It depends on the context, in many cases it's bad but if it's going to be a place that's very likely to have bad input and you have a default to set it to then you would want to catch it.

Upvotes: 0

Related Questions