8

I was recently told that I'm abusing exceptions to control the flow in my applications, so I this is my attempt to somehow clarify the situation.

To my mind, a method should throw an exception, when it encounters a situation, which can't be handled internally or might be handled better by the calling side.


So - does any particular set of rules exist, which can be used to answer the following set of question when developing your applications:

  • When should I throw an exception and when should I write code with strong nothrow guarantee, which might simply return bool to indicate success or failure?

  • Should I try to minimize the number of situations, when the method throws an exception or , on the contrary, should it be maximized to provide flexibility when handling these situations?

  • Should I stick to the exception throwing convention set by the frameworks / runtimes I use when developing my applications or should I wrap all these calls so that they match my own exception throwing strategy?

  • I was also adviced to use error codes for error handling, which seems pretty efficient, but ugly from the syntactical point of view (also, when using them a developer loses the ability to specify the output for a method). What do you think about this?


Example for the third question (I was using an I/O framework and encountered the following situation):

The described framework does not use exceptions to handle errors, but the other code does use them. Should I wrap every possible failure indicated with '???' and throw an exception in this case? Or should I change the signature of my method to bool PrepareTheResultingOutputPath and only indicate whether the operation was successful or not?

public void PrepareTheResultingOutputFile(
    String templateFilePath, String outputFilePath)
{
    if (!File.Exists(templateFilePath))
        // ???

    if (!Directory.MakePath(outputFilePath))
        // ???

    if (File.Exists(outputFilePath))
        if (!File.Remove(outputFilePath))
            // ???

    if (!File.Copy(templateFilePath, outputFilePath)
        // ???
}

Another example - even the .NET Framework doesn't follow some strict exception throwing strategy. Some methods are documented to throw 10+ different exception types, including trivial exception types like NullArgumentException, but some of them simply return bool to indicate success or failure of the operations.

Thank you!

Cocowalla
  • 11,717
  • 5
  • 58
  • 93
Yippie-Ki-Yay
  • 20,062
  • 23
  • 85
  • 143
  • 2
    Possible duplicates: http://stackoverflow.com/questions/729379/why-not-use-exceptions-as-regular-flow-of-control http://stackoverflow.com/questions/99683/which-and-why-do-you-prefer-exceptions-or-return-codes http://stackoverflow.com/questions/345626/how-can-i-avoid-using-exceptions-for-flow-control http://stackoverflow.com/questions/1336094/using-try-catch-for-flow-control-net http://stackoverflow.com/questions/465953/throwing-exceptions-to-control-flow-code-smell – Cocowalla Feb 16 '12 at 08:09
  • An interesting restriction to give yourself is to write code without getters/return values. Just tell objects to do something. Don't expect an error code. Where you would normally take action based on an error code, put that behaviour on the object instead of returning the error code. Maybe pass in an error handler, for example, so that the class can do the right thing. Only then should you throw an exception if your class has been unable to meet its responsibility. – Peter Wood Feb 16 '12 at 08:12
  • 1
    Error codes can be accidentally ignored. In languages that support exceptions, error codes are anachronisms. – Andy Thomas Feb 17 '12 at 15:17
  • See also item 72, "Prefer to use exceptions to report errors", in "C++ Coding Standards" by Sutter and Alexandrescu, for a comparison of exceptions and error codes. – Andy Thomas Feb 17 '12 at 15:27

5 Answers5

4

The problem with exceptions is that they are essentially glorified gotos that have the ability to unwind the program's call stack. So, if you are "using exceptions for flow control," you are probably using them as gotos rather than as indications of an exceptions condition. That's exactly the point of exceptions, and the reason for their name: they are supposed to be used only in exceptional cases. So, unless a method is designed not to throw an exception (an example is .NET's int.TryParse), it's OK to throw an exception in response to exceptional circumstances.

The nice thing about C# as opposed to Java is that in C# you can essentially return two or more values, by returning a tuple type or by using out parameters. So, there isn't much ugliness in returning an error code as the method's main return value, since you can use out parameters for the rest. For example, the common paradigm for calling int.TryParse is

string s = /* Read a string from somewhere */;
int n;
if (int.TryParse(s, out n))
{
    // Use n somehow
}
else
{
    // Tell the user that they entered a wrong number
}

Now for your third question, which seems to be the most substantial. In reference to your example code, you ask if you should return bool to indicate success/failure or if you should use exceptions to indicate failure. There is a third option, though. You can define an enum to tell how the method could fail, and return a value of that type to the caller. Then, the caller has a wide choice: the caller doesn't have to use a bunch of try/catch statements, or an if that gives little insight into how the method failed, but can choose to write either

if (PrepareTheResultingOutputFile(templateFilePath, outputFilePath) == Status.Success)
    // Do  something
else
    // It failed!

or

switch (PrepareTheResultingOutputFile(templateFilePath, outputFilePath))
{
    case Status.Success:
        // Do something
        break;
    case Status.FileNotPresent:
        // Do something else
        break;
    case Status.CannotMakePath:
        // Do something else
        break;
    // And so on
    default:
        // Some other reason for failure
        break;
}

You can find more on this issue here and here, but especially in Joel Spolsky's post, which I highly recommend.

Community
  • 1
  • 1
Adam Mihalcin
  • 13,346
  • 3
  • 29
  • 49
  • Thank you very much for your answer and your references. However, as I realize, there is not much difference between using error codes and `Status` enum *(because this enum acts purely as the "action-specific" error code)*. Even more, if you design a library using this approach, any attempt to add another operation status might become a breaking change. Basically, developers could end up with the code, which simply ignores certain types of errors and that is a step towards undefined behavior. – Yippie-Ki-Yay Feb 16 '12 at 16:57
  • @Yippie-Kai-Yay Yes, there is little difference between a `Status` enum and an exit code. The only difference is that an enum is much safer and more expressive when written in code. For your second point: adding another operation status is indeed a breaking change (although I edited the switch statement in my answer to include a default, which stops the change from causing a break), but so is adding another exception. If you suddenly start throwing another exception, you again break callers that aren't catching that exception. Future proofing is a generally hard problem. – Adam Mihalcin Feb 16 '12 at 22:32
  • The "glorified goto" claim seems weak. Branching and looping constructs require jumps as well. – Andy Thomas Feb 17 '12 at 15:25
  • @AndyThomas-Cramer Whether branching and looping constructs, and indeed gotos and exceptions, are implemented with jumps is an implementation detail of the compiler and processor. The whole reason that goto is considered harmful is that it obfuscates the control flow in the code. A basic for loop clearly says "execute this code N times," and an if says "based on the value of this expression, execute one of two blocks of code." An exception says: "if we reach an exceptional case and throw an exception, go to the caller's catch." So exceptions are harder to read than loops and branches. – Adam Mihalcin Feb 17 '12 at 18:54
  • @Adam - Unbridled gotos were indeed obfuscating. Exceptions are not unbridled gotos. They are just a new structured flow of control. They are not universally considered obfuscating. And they are usually considered a better solution for indicating that a function has failed than error codes. In short, ""Exceptions considered harmful" considered harmful". – Andy Thomas Feb 18 '12 at 13:54
  • @AndyThomas-Cramer I agree that they are not always obfuscating, but my whole answer argues that they, and the detriments they bring to readability (and although the answer doesn't mention it, performance), should be used, and not abused, for exceptional cases only. – Adam Mihalcin Feb 18 '12 at 18:40
2

There's nothing inherently evil about exceptions. When properly used they can greatly ease error handling in your code. The problem with exceptions, particularly in Java, is that they are to easily abused and overused, leading to a variety of anti patterns.

As to your specific questions, I will provide my own opinion on each one.


When should I throw an exception and when should I write code with strong nothrow guarantee, which might simply return bool to indicate success or failure?

You cannot write a method in Java with a 'no throw' guarantee. At minimum the JVM can throw a runtime error at any time, say for example an OutOfMemoryError. It's not your responsibility to suppress these, just let them bubble up your call heirarchy till you reach the most appropriate location to handle them. Changing a method's return type to bool to indicate success or failure is actually an antithesis of good design, your methods return type should be dictated by their contract (what they are supposed to do), as opposed to how they did it.


Should I try to minimize the number of situations, when the method throws an exception or , on the contrary, should it be maximized to provide flexibility when handling these situations?

Neither! Your method should throw exactly the number of exceptions as expected, given its contract (i.e what its supposed to do). Here are some general rules:

  1. Its not your method's responsibility to handle exceptions that occur as a result of actions it does not take. I.e, OutOfMemory or StackOverflow error's thrown by the JVM
  2. It is the responsibility of the method to handle all exceptions, thrown as a part of its execution, that result from deferred calls to other modules that are not explicitly visible to the method's caller. So for example, if you use the Apache Commons IO library to process input streams, in a method designed to read a file, you need to handle any exceptions the library throws. This is because the method's caller has no way of knowing you are using this library within your method. The most typical manner of handling these sorts of exceptions are by rewrapping them in some instance of a runtime (unchecked) exception. You can also wrap these in a checked exception, if you want a clear indication to the methods caller that it needs to be prepared to handle exceptional circumstances.
  3. It is the responsibility of the method to throw an exception (checked or unchecked) if for any reason it is not able to fulfill its contract (aka, it cannot complete successfully). Case to point, each one of the conditional (if) statements in your PrepareTheResultingOutputFile method is a valid point to thrown an exception on failure of the desired result.

Should I stick to the exception throwing convention set by the frameworks / runtimes I use when developing my applications or should I wrap all these calls so that they match my own exception throwing strategy?

If both the method and method caller are using the same framework then it is totally unnecessary to wrap the frameworks exceptions before rethrowing them. The converse is also true - if you are using a framework in your method that your caller does not know about, then you should hide that implementation detail by wrapping the exceptions the framework throws.


I was also adviced to use error codes for error handling, which seems pretty efficient, but ugly from the syntactical point of view (also, when using them a developer loses the ability to specify the output for a method). What do you think about this?

I have not seen a lot of successful error code frameworks in Java, and to be honest in most cases its total overkill. A greater argument could be made for internalization and localization of error messages.

Perception
  • 75,573
  • 19
  • 170
  • 185
  • I would add that nowhere in your program should you handle exceptions like `OutOfMemoryError` or `StackOverflowException`, except maybe to show an error dialog to the user and shut down the program. Even that is dicey, since if you're out of memory, how are you supposed to create a new error dialog and show it to the user? – Adam Mihalcin Feb 17 '12 at 19:01
1

Using exception for errors which are, well, exceptional is OK. For example, throwing an exception if memory allocation fails in a hosted environment is OK ( in an embedded enivronment it may better be dealt with differently). Likewise, throwing if the contract isn't observed (e.g. throwing when receiving a null pointer where a valid pointer is expected) is probably reasonable (as may be aborting). Using exceptions for expected errors or control flow will work but mess up any hope for acceptable performance.

Dietmar Kühl
  • 141,209
  • 12
  • 196
  • 356
1

I feel the first question have been answered, and is quite simple. Only use exceptions in exceptional situations.

For your other questions:

Should I try to minimize the number of situations, when the method throws an exception or , on the contrary, should it be maximized to provide flexibility when handling these situations?

If I understand your question correctly, it kind of answers itself by question 1. The situations when you throw an exception are in exceptional situation, which of course is not a lot of situations. You should make sure your program almost never hit an exception, except when the Sun and Saturn are aligned. You should however have test cases which tests that exceptional situations actually throws exception.

Should I stick to the exception throwing convention set by the frameworks / runtimes I use when developing my applications or should I wrap all these calls so that they match my own exception throwing strategy?

Depends. In your example it depends if the not finding a file is exceptional? If you expect this file to exist, while it doesn't (for example a file installed alongside your program) it should throw an exception. If it's a file the user wish to open, you can't be sure and have to account for this. I don't think a user error of this sort is exceptional, and would probably use a return code for this. Other things to consider is: Is the success of this critical for the execution of the program? Does calling this function with that parameter violate the contract? The answer to your question is not straight forward, and you have to do it on a case by case basis. Again: Is this an exceptional situation?

I was also adviced to use error codes for error handling, which seems pretty efficient, but ugly from the syntactical point of view (also, when using them a developer loses the ability to specify the output for a method). What do you think about this?

Error codes is efficient, but might make your code harder than necessary to read. Exceptions are a really nice way to handle this, but might be inefficient. If you don't make a performance critical part of your program though, I wouldn't care to much about that.

martiert
  • 711
  • 4
  • 13
0

As you said if there's an internal situation that can't be handled an exception should be thrown but if there's an external problem (a call to another module failed) you should let the module itself handle the situation unless the exception that's thrown by the module is meaningless regarding to the context then you should wrap the exception with a meaningful one.This way you shouldn't throw exceptions for every possible error that might happen.sometimes a simple "Connection Time-out" exception from the data access layer is suffice and there's no need to wrap it with another excption.

To demonstrate what I mean here's an example

var pop3Clinet=new pop3Client();

    try
    {
    pop3Client.SetServer("server-address");
    pop3Client.SetUserName("username");
    pop3Client.SetPassword("password");
    var mails=pop3Client.ReceiveMails();
    }
    catch(NullReferenceException exp)
    {
     throw new Exception("can not connect to server.server-address is wrong or the server is down",exp);
    }
    catch(UnauthorizedAccess)  //the exception is meaningful and can be rethrown or this block can be removed.
    {
    throw;
    }

This example is a real one :) this pop3client that we use strangely fail to throw a server related exception every time it tries to connect to a server so we had to wrap NullReference exception into a meaningful one.

Returning bool to show that a method has been successful is not recommended unless it's the reason that the method has been written.for example in .net we have some TryParse methods of primitive types such as int , long and etc that return a bool if they are successful and that's what they do , they try to do something and if they don't success they report it back. In this scenario you simply don't care what went wrong and why cause most of the times the provided data is in a wrong format to parse. in another word you can use such kind of methods to check the situation to take everything under your control. but if you have a method that takes a list of integers in string format and try to do a calculation over them then you should throw an exception if your method is unable to parse some of these values.You should even report which one does not have the right format.

Beatles1692
  • 4,814
  • 27
  • 60