views:

98

answers:

3

Hi guys,

This is a refactoring question.

try
{
  string line = GetFirstLineFromFile(); //Gets first line from a text file, this line would be a number.
  int value = ConvertToInteger(line); // Gets the integer value from the string.
  int result = DivideByValue(value); // Divides some number with the value retrieved.
}
catch(Exception ex)
{
}

My main concern is, what is the best approach for exception handling in such situations. Certainly wrapping the whole thing in a single try catch is like saying I expect an exception about everything. There must be some place we catch a generic exception right?

+2  A: 

Just don't catch a "generic exception".

How can you possibly handle ANY exception and know how to keep your application in a clean state ? It hides bugs and it's a really bad idea.

Read this serie of posts on catch (Exception).

Guillaume
I do know that I would not write such a code. But say I remove the all wrapping try catch block, each of those functions would have some kind of exception handling going on in them. Is it feasible to cover all the expected exceptions and write catch blocks for them?
theraneman
The methods you call should have a defined set of exception thrown.Then just catch only the defined exceptions...
Guillaume
+1  A: 

You need to think about what exceptions can be thrown from the methods in the try block, as well as which ones of those you can deal with at the current level of abstraction.

In your case, I'd expect that the getFirstLineFromFile methods, for example, can definitely throw exceptions you'd want to catch here. Now whether you wrap and rethrow the exception, or take other action, depends on whether you can actually deal with the exception at this level. Consider the case where you have a default file you can fall back to - the approach may just be to log a warning and continue with the default. Or if the whole application is based on reading a file supplied by the user, then this is more likely to be a fatal exception that should be propagated up to the top level and communicated to the user there.

There's no hard-and-fast rule like "always throw" or "never throw"; in general, I consider that one should throw exceptions whenever there's an exceptional-type situation that is not considered a normal result of the method, and consequently cannot be adequately described by the return type of the method. (For example, an isValidDbUser method returning boolean might be able to handle SQLExceptions as just return false; but a getNumProductsRegisteredInDB returning an int should almost certainly propagate an exception).

Andrzej Doyle
Having read the CLR via C#, exception handling is certainly not as straight forward as folks use it. Can you tell when would you catch the generic Exception? What I have been doing until now is add catch statements for specific exceptions and then have a generic exception catch block.
theraneman
Why would you catch a generic Exception? What could you do with it? In my mind, the *only* time you can legitimately do this is at the very top level of your program/webapp (or possibly non-essential operation), in order to translate it into an appropriate response for the user. In the case of the webapp, at least, you don't want the whole app to come down. Otherwise, use a `finally` block if you want to be sure to execute cleanup code.
Andrzej Doyle
A: 

Don't listen to the (hordes) of people that will tell you that you should never catch multiple exceptions in one big general block. It's a perfectly reasonable way to do general error handling in some cases, which is why the ability to do so exists in the language.

There will be some exceptions that you can do something specific and useful about (i.e. recover from them in the catch block.) These are the kinds of exceptions that you want to catch individually, and as close to the place where they occur as possible.

But the majority of exceptions that you'll face in real life will be completely unexpected, unchecked exceptions. They are the result of programmer error (bugs), failed assertions, failing hardware, dropped network connections, etc.

You should design your software defensively, by designating specific "chokepoints" to handle these unpredictable exceptions with a minimum of disruption to the rest of the application. (Remember, in many cases, "handling" the exception often just means aborting the current operation and logging an error or otherwise telling the user that an unexpected error occurred.)

So for example, if your program saves a file to the disk, you could wrap the entire save operation in a try block to catch things that goes wrong during the save:

try {
   // attempt to perform the save operation
   doSave();

} catch (Throwable t) {
   // tell the user that the save failed for unexpected reasons
   // and log the error somewhere
   informUser("save failed!");
   log("save failed!", t);

} finally {
   // last minute cleanup (happens whether save succeeded or failed)
   ...
}

Notice that we choose a nice chokepoint method here ( doSave() ) and then stop any unexpected errors from bubbling up any further than this point. The chokepoint represents a single, cancellable operation (a save). While that operation is obviously toast if you're getting an unexpected exception, the rest of the application will remain in a good state regardless of what happens on the other side of the chokepoint.

Also notice that this idiom does NOT stop you from handling some of your exceptions further down in doSave() somewhere. So if there are exceptions that might get thrown that you can recover from, or that you want to handle in a special way, go ahead an do so down in doSave(). But for everything else, you have your chokepoint.

You might even want to set up a general uncaught exception handler for your entire program in your main method:

public static void main(String [] args) {
    try {
       startApplication();
    } catch (Throwable t) {
       informUser("unexpected error! quitting application");
       log("fatal application error", t);
    }

But if you've set your other chokepoints up wisely, no exceptions will ever bubble up this far. If you want to make your general error handling complete, you can also create and assign an UncaughtExceptionHandler to important threads, including your main thread or the AWT thread if you are using Swing.


TL;DR; Don't believe the dogma that you should always catch exceptions as specifically as possible. There are times when you want to catch and handle a specific exception, and other times when you want to use chokepoints to catch and deal with "anything else that might go wrong".

Xanatos
Precisely. Catching specific exceptions does say that the developer thinks these particular exceptions might occur. Although in front end applications where there needs to be a friendly error message to the user in case something unexpected goes wrong, a generic exception saves the day.
theraneman