10

I've written a function that asks a user for input until user enters a positive integer (a natural number). Somebody said I shouldn't throw and catch exceptions in my function and should let the caller of my function handle them.

I wonder what other developers think about this. I'm also probably misusing exceptions in the function. Here's the code in Java:

private static int sideInput()
{
    int side = 0;
    String input;
    Scanner scanner = new Scanner(System.in);

    do {
        System.out.print("Side length: ");
        input = scanner.nextLine();
        try {
            side = Integer.parseInt(input);
            if (side <= 0) {
                // probably a misuse of exceptions
                throw new NumberFormatException();
            }
        }
        catch (NumberFormatException numFormExc) {
            System.out.println("Invalid input. Enter a natural number.");
        }
    } while (side <= 0);

    return side;
}

I'm interested in two things:

  1. Should I let the caller worry about exceptions? The point of the function is that it nags the user until the user enters a natural number. Is the point of the function bad? I'm not talking about UI (user not being able to get out of the loop without proper input), but about looped input with exceptions handled.
  2. Would you say the throw statement (in this case) is a misuse of exceptions? I could easily create a flag for checking validity of the number and output the warning message based on that flag. But that would add more lines to the code and I think it's perfectly readable as it is.

The thing is I often write a separate input function. If user has to input a number multiple times, I create a separate function for input that handles all formatting exceptions and limitations.

usr
  • 201
  • 1
  • 2
  • 3

6 Answers6

11

The point of an exceptions is that it allows a method to tell that caller that entered a state where it could not continue normally, without forcing you to embed an error code in the return value.

In your case, your method knows exactly what to do when the input is not greater than 0. The only reason you are saving lines here is because you happen to be throwing the same exception you would get if the input was not a number. However, the exception you are throwing does not properly represent why your code does not like the input. If someone else were to come along and see this code, they would have to spend extra time trying to see exactly how things are working.

6

This is a bad use of exceptions. To begin with, a non-positive number is not a format exception.

Why use exceptions at all? If you know what input is not allowed, just don't break out of the loop until you get valid input from the user, something like the following:

while (true)
{
   // Get user input.
   String input = scanner.nextLine();

   try
   {
      side = Integer.parseInt(input);

      break;
   }
   catch (NumberFormatException ex)
   {
      // Inform user of invalid input.
      System.out.println("Invalid input. Enter a natural number.");
   }
}
Bernard
  • 8,869
2

You should not both throw and catch the same exception in a method, I even think the the catch block will catch the same exception you are throwing, so you are not really throwing it.

If parseInt was successful, then it's not a NumberFormatException.

if side is less than zero, you should throw a NegativeSideLengthException;

Create a custom/bussiness Exception named NegativeSideLengthException

public class NegativeSideLengthException extends Exception
{


    public NegativeSideLengthException(Integer i)
    {
        super("Invalid negative side length "+i);        
    }

}

Then sideInput throws NegativeSideLengthException

private static int sideInput() throws NegativeSideLengthException
{
    int side = 0;
    String input;
    Scanner scanner = new Scanner(System.in);

    do {
        System.out.print("Side length: ");
        input = scanner.nextLine();
        try {
            side = Integer.parseInt(input);
            if (side <= 0) {
                throw new NegativeSideLengthException(side);
            }
        }
        catch (NumberFormatException numFormExc) {
            System.out.println("Invalid input. Enter a natural number.");
        }
    } while (side <= 0);

    return side;
}

You can even ( if you want ) add another catch block to catch NegativeSideLengthException and not have the method throw it.

do {
    System.out.print("Side length: ");
    input = scanner.nextLine();
    try {
        side = Integer.parseInt(input);
        if (side <= 0) {
            throw new NegativeSideLengthException(side);
        }
    }
    catch (NumberFormatException numFormExc) {
        System.out.println("Invalid input. Enter a natural number.");
    } catch (NegativeSideLengthException e){
        System.out.println("Invalid input. Enter a non-negative number.");
    }
} while (side <= 0);

Flags are not a good way to handle exceptions.

Tulains Córdova
  • 39,570
  • 13
  • 100
  • 156
1

Only catch exceptions if you intend to do something that is relevant to the current method call; i.e cleanup, failure logic etc. In this case, the catch is merely sending a message to the console, it is not relevant to sideInput method, so therefore it can be handled further up the call chain/stack.

One can get rid of the try/catch here and simply document the method call:

//Throws NumberFormatException if read input is less than 0
private static int sideInput()

One still needs to handle that exception further up the call chain/stack!

Jon Raynor
  • 11,773
1

There’s the general case, and your specific case.

A general rule “you mustn’t throw and catch in the same method” is nonsense. If your function detects an exceptional situation then it throws an exception. If it catches exceptions (and handles them properly) you catch exceptions. If your function does both you do both. It is slightly unusual, but that doesn’t matter.

Now in your particular case: Non-positive numbers are not an exceptional situation. Therefore you shouldn’t throw an exception. And you are throwing the wrong exception. Better to have two different error messages for situations where the parser throws an exception, and for non-positive input.

And your code is quite complex. What would happen if you didn’t initialise “side” or initialised it to +1? Or you changed the requirement to side >= 0?

gnasher729
  • 49,096
-1

Exceptions are pretty nightmarish things, they bring more complexities than they solve.

Firstly, if you don't catch your exceptions, the caller can only do on error resume next, that is, after one week, even you won't know what your function can throw, and what to do with it:

{
    ...
}
catch(OutOfMemory, CorruptedMemory, BadData, DanglingPointers, UnfinishedCommit)
{
    Console.WriteLine("Nothing to see here, move on.");
    Console.WriteLine("The app is very stable, see, no crashing!");
}

Basically, if you catch them, you have to have a very good understanding about contracts, and exception guarantees. That rarely happens in a real world. And also your code will be hard to read.

Also, the funny thing is that if you really have any chance of handling exceptions you need real RAII language, which is sort of humorous, since Java and .NET is all about exceptions...

Repeating this one more time, but...:

Link

Link

http://www.joelonsoftware.com/items/2003/10/13.html

Glorfindel
  • 3,167
Coder
  • 6,978