Wrong usage of exception - return value from catch

Question

This is the piece of code.

  private boolean CheckTerm()
      {
          String str = lGskCompoundNumber;
          if (lPrefix.trim() == "" || lNumber.trim() == "")
              return false;

          try
          {
              Integer.parseInt(lNumber);
          }
          catch (Exception ex)
          {
              return false;
          }
          if (lHasAmpersand)
              str = lGskCompoundNumber.replace("&", "");
          return str.equals(lPrefix + lInitSep + lNumber + lEndSep + lSuffix);
      }

Should I return a certain value from catch block or is the usage right?

What do you mean by "Should I return a certain value from catch block ", return to where? — Adriaan Stander, Jul 26 '12 at 07:47
I think he's asking if you should use the `return` keyword in a `catch` block. I think that's fine. — Autar, Jul 26 '12 at 07:50

score 10 · Accepted Answer · answered Jul 26 '12 at 07:51

This code is correct and does not look suspicious. When parsing fails (note that you should catch the most narrow exception possible, NumberFormatException in this case), whole validation failed, so you are returning false. Otherwise you are performing additional checks (after catch block) and returning their result. This code is fine.

If you find it a bit hard to read, consider the following refactoring:

private boolean checkTerm() {
    try
        {
            String str = lGskCompoundNumber;
            if (lPrefix.trim() == "" || lNumber.trim() == "")
                return false;
            Integer.parseInt(lNumber);
            if (lHasAmpersand)
                str = lGskCompoundNumber.replace("&", "");
            return str.equals(lPrefix + lInitSep + lNumber + lEndSep + lSuffix);
        }
        catch (NumberFormatException ex)
        {
            return false;
        }
    }

score 3 · Answer 2 · answered Jul 26 '12 at 07:59

There's nothing wrong in your code but I always prefer to have a single exit point in my methods, so I prefer to write something like:

private boolean CheckTerm()
{
    boolean res = false;
    String str = lGskCompoundNumber;
    if (lPrefix.trim() == "" || lNumber.trim() == "")
    {
    }
    else
    {
        try
        {
            Integer.parseInt(lNumber);
            if (lHasAmpersand)
                str = lGskCompoundNumber.replace("&", "");
            res = str.equals(lPrefix + lInitSep + lNumber + lEndSep + lSuffix);
        }
        catch (NumberFormatException ex)
        {
        }

    }
    return res;
}

score 2 · Answer 3 · answered Jul 26 '12 at 07:56

I believe that it is okay if you know what you want to return. If I could give a hint on the number of exits(returns) you use. I would use a boolean field and just return once after the try / catch Block

private boolean CheckTerm()
{
  boolean b = true;
  String str = lGskCompoundNumber;
      if (lPrefix.trim() == "" || lNumber.trim() == "")
          b = false;

      try
      {
          Integer.parseInt(lNumber);
      }
      catch (Exception ex)
      {
          b =  false;
      }
      if (lHasAmpersand)
          str = lGskCompoundNumber.replace("&", "");
      b = str.equals(lPrefix + lInitSep + lNumber + lEndSep + lSuffix);
      return b;
  }

score 1 · Answer 4 · answered Jul 26 '12 at 07:52

1

you can return some value in catch block, and even in finally block.

It's totally okay.

answered Jul 26 '12 at 07:52

meadlai

895
1
9
22

score 1 · Answer 5 · answered Jul 26 '12 at 07:52

1

If one of the conditions of CheckTerm is that lNumber should be an integer, then your code is ok, but if you're expecting lNumber to be an integer and it's not, you'll better throw an exception.

answered Jul 26 '12 at 07:52

tibtof

7,857
1
32
49

score 1 · Answer 6 · edited May 23 '17 at 12:34

1

Its fine as is though I would catch the exception differently.

That method your calling throws a NumberFormatException

it seems as though just returning false would be fine, i would do some sort of logging to keep track of the exception though discussion on how to handle your exception

edited May 23 '17 at 12:34

Community

1
1

answered Jul 26 '12 at 07:53

Frank Visaggio

3,642
9
34
71

score 1 · Answer 7 · answered Jul 26 '12 at 08:06

if you afraid there is no default returns, using finally, then return something.

try
    {
        Integer.parseInt(lNumber);
        if (lHasAmpersand)
            str = lGskCompoundNumber.replace("&", "");
        res = str.equals(lPrefix + lInitSep + lNumber + lEndSep + lSuffix);
    }
    catch (NumberFormatException ex)
    {
       return false;
    }finally{return false;}

Wrong usage of exception - return value from catch

7 Answers7