Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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.

share|improve this question
    
Highly depends on language. Some languages use exceptions more freely than others. –  Loki Astari Nov 22 '11 at 21:17
add comment

5 Answers 5

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.

share|improve this answer
    
Yeah, that's why I thought it was a misuse. So, ignoring that throw statement, you agree that it's OK to handle exceptions in such loop inside a function instead of letting the caller catch them? The catch statement is there because of Integer.parseInt (again, ignoring the throw). –  usr Nov 22 '11 at 20:39
1  
@usr: That answer depends much more on how the rest of the system is meant to work together. Exceptions need to be handled at some point. There are a few different ways you could organize it and this is one of them. Which is best depends on other information we don't have here. –  unholysampler Nov 22 '11 at 20:51
add comment

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.");
   }
}
share|improve this answer
    
Integer.intParse throws a format exceptions, so using the catch statement is perfectly valid. I mainly want to know if it's OK to use catch statement in a loop in a function or should I let the caller of the function handle format exception. –  usr Nov 22 '11 at 20:47
    
Integer.parseInt() throws a NumberFormatException if it can't parse the provided string argument. There is no need for you (and you won't be able) to throw the exception yourself. –  Bernard Nov 22 '11 at 20:51
    
I have edited the code example to be more explicit. –  Bernard Nov 22 '11 at 20:56
    
"and you won't be able) to throw the exception yourself" - what do you mean there? –  Michael Borgwardt Nov 22 '11 at 21:24
    
@Michael Borgwardt: I mean that since the Integer.parseInt() method will throw the exception for you if it occurs, you won't be able to throw it yourself afterward since it has already been thrown. –  Bernard Nov 22 '11 at 21:58
add comment

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!

share|improve this answer
1  
The message is there just for a better UI. The point of the function is that it nags the user for input until she/he enters a valid input. This can't be done without try-catch blocks. My question was if the point itself is valid. I think it is, but someone told me I should remove the try-catch blocks and let the caller handle this particular exception. But then the function won't work as intended. –  usr Nov 25 '11 at 13:11
add comment

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.

share|improve this answer
add comment

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

http://blogs.msdn.com/b/oldnewthing/archive/2004/04/22/118161.aspx

http://blogs.msdn.com/b/oldnewthing/archive/2005/01/14/352949.aspx

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

share|improve this answer
4  
-1 for posting a borderline rant, full of not quite correct -or at least context/language dependent - statements, instead of answering the OP's actual question. –  Péter Török Nov 22 '11 at 22:20
    
@PéterTörök: "Give a man a fish and you feed him for a day. Teach a man to fish and you feed him for a lifetime." There are very serious problems behind exceptions, and people should know them -1000/+1, I don't really care. –  Coder Nov 23 '11 at 16:59
1  
Feel free to believe you can write correct code easier without than with exceptions. Just don't state your views and beliefs as facts (even if Joel is of the same opinion, it is still an opinion, not a fact), and don't post irrelevant answers on SE. –  Péter Török Nov 23 '11 at 18:46
    
@PéterTörök: It's not an opinion, it's a fact, every time you use a component that throws an exception internally, you have to crawl the whole hierarchy and check every single line of code to know what to catch, and if the components offer strong guarantee, and everything is rolled back, or if that catch is just a false sense of security. Heck, you don't even know all the exceptions std::string throws, you can look up the specs, but you'll never find. Things like : throw(thisexception, thatexception) are downright wrong and should never be used, because otherwise you'll get unexpectedexcept. –  Coder Nov 23 '11 at 22:02
2  
OK, so how about code not using exceptions? Gee, you need to crawl through the code to check every single line to see whether return values are handled properly or ignored. And when a return value is ignored, noone notices until maybe your app crashes a couple thousand lines later. Exceptions at least force you to take notice. Yes, you can swallow them - but only with an explicit catch. While an ignored return value is unsearchable - it is identifiable only via line-by-line code review. Last but not least, constructors have no return values, this is the foremost reason to use exceptions. –  Péter Török Nov 24 '11 at 8:36
show 2 more comments

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.