Is catching NumberFormatException a bad practice?

Question

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.

Hadron

This isn't necessarily bad practice, but it may help to show us what the input looks like. IMO, you can never do enough error checking. – Gooseneck

I think it's ugly and difficult to read, but there isn't a better solution. That's one of the reasons I don't like checked exceptions c2.com/cgi/wiki?TheProblemWithCheckedExceptions – Ailment

You might find this question is relevant. – Doyle

Did you ever resolve this issue? – Linda

@AlexLockwood I was still hoping for new answers...with no luck :) So i will wrap the exception in a method boolean isHex(String str), just for clarity :) – Hadron

@integeruser sounds good. this sounds like something you don't have to worry too much about anyway... just do it whichever way you think looks best :) – Linda

Answer 1

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;
    }
}

Linda

@integeruser, if a wrong input doesn't exist, then of course your code is ok as written above... in fact, i don't believe there is any other possible way you can avoid a program crash without handling the NumberFormatException... can you please clarify what the problem is? – Linda

i have to check if a string is a hex number or not. Yes my code works, but i find it ugly and i don't think it is the best approach. See pb2q's answer. – Hadron

@integeruser, I agree with you in that the "exception handling" aspect of it is can look ugly, but there is no way to avoid it in most situations. one way you can make your code a bit prettier (as pp2q described as well) is by wrapping your code in a helper method of some sorts. that way you separate the functionality of your methods... methods that serve one purpose won't be all cluttered up with exception catching/handling. not sure if i am making sense right now, but hopefully im getting my point across haha – Linda

i agree with you :) and i think i will wrapper the exception in a method. What i find unreasonable is that these methods like isInt or isHex aren't already included in some Java classes :P – Hadron

Yes, well... fortunately for you the methods provided in the NumberUtils class are extremely simple, for the most part. I wouldn't even go as far as importing these classes into your project... I'd just copy and paste the methods you need directly into your code, or in some sort of utility class, etc. Let me know what you end up doing :) – Linda

Answer 2

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:

• At what level and how is appropriate to catch exceptions

• Why should I not wrap every block in "try"-"catch"?

Rollins

I don't agree that it's 'definitely' better to catch more explicit exceptions. If some method can throw a number of exceptions and you can't adequately recover from any of them, then why bother with anything other than catching 'Exception' (or whatever) and logging it. I do agree with the 2nd quoted item, "a method should only catch an exception..." – Preachment

I don't think that your answer really addresses the question, which isn't about how/when should I use exceptions, but about using Exceptions when it might be more appropriate to use e.g. if/else. – Moravian

Answer 3

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.

Linda

By the way, for those of you who develop for Android, I know you should use the android:inputType attribute instead... don't hate. – Linda

Answer 4

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.

Moravian

"Exception handling is Good". The other person who asked earlier today explictly asked if simply having a try/catch block could be considered "ugly". No! Nothing could be further from the truth. In this particular case, I strongly believe an exception handler is both appropriate ... and ideal. For several different reasons. IMHO... – Rollins

PS: I agree with your statement about "exceptional conditions". Some people think exceptions should only be used for "errors". No: they can usefully be used in any scenario that doesn't conform to "the standard flow". Good point ;)! – Rollins

Answer 5

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.

Faldstool

Answer 6

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...

Planar

I suppose your first statement is true, however, almost all code used in a professional environment will have bugs either initially, or as it is added to. Thus, how good your code looks does matter, or rather, how readable your code is matters. – Newmodel

Answer 7

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.

Chinchilla