Exceptions for flow control in C#

By Vladimir Khorikov

The use of exceptions for flow control was raised quite a few times already (here’s a c2 discussion and here is a great question on SO). I’d like to summarize this topic and provide some common use cases along with code examples to handle them.

Exceptions for flow control: why not?

Generally, code is read more often than written. Most of the best practices aimed to simplify understanding and reasoning about the code: the simpler code, the fewer bugs it contains, and the easier it becomes to maintain the software.

The use of exceptions for program flow control hides the programmer’s intention, that is why it is considered a bad practice.

public void ProcessItem(Item item)

{

    if (_knownItems.Contains(item))

    {

        // Do something

        throw new SuccessException();

    }

    else

    {

        throw new FailureException();

    }

}

It is hard to reason about ProcessItem function because you can’t say what the possible results are just by looking at its signature. Such approach makes you look at the source code and thus breaks method’s encapsulation. Moreover, it violates the principle of least astonishment by throwing an exception even in case of success.

In this particular example the solution is obvious – return boolean value instead of throwing exceptions – but let’s dive deeper and look at more complex use cases.

Exceptions for input validation

Perhaps, the most common practice for exceptions is to use them to state that input validation is failed.

public class EmployeeController : Controller

{

    [HttpPost]

    public ActionResult CreateEmployee(string name, int departmentId)

    {

        try

        {

            ValidateName(name);

            Department department = GetDepartment(departmentId);

 

            // Rest of the method

        }

        catch (ValidationException ex)

        {

            // Return view with error

        }

    }

 

    private void ValidateName(string name)

    {

        if (string.IsNullOrWhiteSpace(name))

            throw new ValidationException(“Name cannot be empty”);

 

        if (name.Length > 100)

            throw new ValidationException(“Name length cannot exceed 100 characters”);

    }

 

    private Department GetDepartment(int departmentId)

    {

        using (EmployeeContext context = new EmployeeContext())

        {

            Department department = context.Departments

                .SingleOrDefault(x => x.Id == departmentId);

 

            if (department == null)

                throw new ValidationException(“Department with such Id does not exist”);

 

            return department;

        }

    }

}

Apparently, such approach has some benefits: it allows you to quickly “return” from any method right to the catch block in the CreateEmployee method.

Now, let me demonstrate you another code sample:

public static Employee FindAndProcessEmployee(IList<Employee> employees, string taskName)

{

    Employee found = null;

 

    foreach (Employee employee in employees)

    {

        foreach (Task task in employee.Tasks)

        {

            if (task.Name == taskName)

            {

                found = employee;

                goto M1;

            }

        }

    }

 

    // Some code

 

    M1:

    found.IsProcessed = true;

 

    return found;

}

What do these two code examples have in common? Yep, both of them allows you to easily break through the code to the point where you need to appear leaving irrelevant code paths behind.

The only problem with such code is that it drastically decreases readability. Both of the code samples make it really difficult to follow the program flow. That is why many developers often equalize exceptions with the “goto” statement.

With exceptions, it is unclear where exactly they are being caught: you can wrap up your validation routine with a try/catch statement right when you call it but you can also put a try/catch block couple of levels higher. You can never know for sure if it was made intentionally or not:

public Employee CreateEmployee(string name, int departmentId)

{

    // Is it a bug or the method was placed without try/catch intentionally?

    ValidateName(name);

           

    // Rest of the method

}

The only way to find it out is to analyze the whole call stack. Exceptions used for validation make your code much less readable because they don’t express the developer’s intention clear enough. It’s impossible to look at the code and say what can go wrong and how we react to it.

Is there a better solution? Of course!

[HttpPost]

public ActionResult CreateEmployee(string name, int departmentId)

{

    if (!IsNameValid(name))

    {

        // Return view with error

    }

 

    if (!IsDepartmentValid(departmentId))

    {

        // Return view with another error

    }

 

    Employee employee = new Employee(name, departmentId);

    // Rest of the method

}

Putting all the validations explicitly makes your intention obvious. It’s much easier to see what is going on in this method.

Exceptions are for exceptional situations

So what are the use cases for exceptions? The main goal of exceptions is – surprise! – to state an exceptional situation in your software. Exceptional situation is a situation where you don’t know what to do and the best option you have is terminate the current operation entirely.

Examples of exceptional situations may include database connectivity problems, lack of required configuration files and so on. Validation is no such a situation because validation logic, by its definition, expects incoming data to be incorrect (see my previous post).

Another valid use case for exceptions is code contract violation. You, as the class author, expect its clients to meet the code contracts. The situation in which method’s contract is not getting met is not expected, or, in other words, is exceptional.

How to deal with others’ exceptions?

Whether or not a situation is exceptional depends on a context. A developer of a redistributable library might not know how to deal with database connectivity problems because he can’t see the context in which his library is being used.

There’s not much of what the library developer can do in case database is unavailable so throwing an exception would be an appropriate decision. You can take Entity Framework or NHibernate as an example: they expect that database is always available and if it’s not, they throw an exception.

On the other hand, the developer who uses the library might expect that the database goes off-line from time to time and design their application for database failure. If the database fails, the client application can retry the same operation or display user a message with a suggestion to try again later.

Thus, a situation may be exceptional from a bottom level point of view and at the same time be expected from a client code perspective. How should we deal with exceptions thrown by libraries in such case?

Such exceptions should be caught at the lowest level possible. If they are not, your code will suffer the same drawbacks as the “goto” code sample: it won’t be possible to know where a particular exception is being processed without analysing the whole call stack.

public void CreateCustomer(string name)

{

    Customer customer = new Customer(name);

    bool result = SaveCustomer(customer);

 

    if (!result)

    {

        MessageBox.Show(“Error connecting to the database. Please try again later.”);

    }

}

 

private bool SaveCustomer(Customer customer)

{

    try

    {

        using (MyContext context = new MyContext())

        {

            context.Customers.Add(customer);

            context.SaveChanges();

        }

        return true;

    }

    catch (DbUpdateException ex)

    {

        return false;

    }

}

As you can see at the sample above, SaveCustomer method expects problems with database and intentionally catches all such errors. It returns a boolean value that states the operation status which then is processed by the calling method.

SaveCustomer method has a clear signature which tells us that there can be problems with saving a customer, that we expect them and that you should check the returning value in order to make sure that everything went fine.

There’s a widely known best practice that correlates with all said really well: you shouldn’t wrap such code with a generic exception handler. Generic exception handler basically states that you expect any exceptions that could possibly be thrown and that just can’t be a case.

If you do expect some exceptions, you do it for a very limited range of exception types that you know you can safely handle. Putting a generic exception handler leads to situations where you swallow unexpected exceptions often leaving your application in an inconsistent state.

There’s one situation where generic exception handlers are applicable, thought. You can put one at the topmost level to catch all exceptions that were not handled by your code in order to log them. You shouldn’t in any way try to handle them. All you can do is log the exception and gracefully shut the application down.

Exceptions and the fail fast principle

How often do you see a code like this?

public bool CreateCustomer(int managerId, string addressString, string departmentName)

{

    try

    {

        Manager manager = GetManager(managerId);

        Address address = CreateAddress(addressString);

        Department department = GetDepartment(departmentName);

 

        CreateCustomerCore(manager, address, department);

        return true;

    }

    catch (Exception ex)

    {

        _logger.Log(ex);

        return false;

    }

}

This is an example of incorrect use of generic exception handler. It implies that any exception coming from the method body signalize an error in the process of customer creation. But what is the problem with such approach?

The problem (besides the “goto” issues we discussed earlier) is that an exception might not be an exception we are concerned about. The exception could be of type ArgumentException which we expect, but it also could be ContractViolationException and if it is, then we are hiding a bug by pretending that we know how to deal with such exception.

Sometimes such approach is applied by developers who are willing to protect their software from unexpected failures. But the truth is that this solution masks any potential bugs in your software making it hard to reveal them.

The best way to deal with unexpected exceptions is to discard the current operation completely. If your application is stateless (for example, if you are running a background job), you can just restart the process because in this case you don’t have any data corrupted.

But if your application has a state or there are any chance it could be left with corrupted data, you should log the exception details and crash the process preventing inconsistent behavior from expanding.

Summary

Let’s recap:

  • Throw an exception to state an unexpected situation in your software.
  • Use return values for input validation.
  • If you know how to deal with exceptions a library throws, catch them at the lowest level possible.
  • If you have an unexpected exception, discard current operation completely. Don’t pretend you know how to deal with them.




  • Amir Shitrit

    And, in some cases, an expected exception (such a transient network error) can become an unexpected one (after several retries and/or a circuit-breaker).
    What are your thoughts about these cases?

    • http://enterprisecraftsmanship.com/ Vladimir Khorikov

      I agree. If your application expects some resource being accessible within several retries, inability to reach it out would be an unexpected situation. That should depend on the application’s domain.

  • Luis Masuelli

    The last exception case is almost the Diaper Pattern. OMG I’m laughing again. On the other side: Validation DOES trigger an exception. Usually not when validating, but yes when trying to save, eg: “save!” in RoR.

    Exceptions on record not found are a very common pattern that help us to easily trigger a 404 in common frameworks. Exceptions on validations, a 400. Perhaps the most appropriate common exception case is the PermissionDenied (403).

    But Exceptions for… unexpected? It will depend on what are you EXPECTING. Don’t talk on behalf on every programmer, since readability varies on culture, on programming language, and on framework.

  • Michael G.

    While I agree, and, after falling into the trap of exception flow control myself, have adapted to using a Result object instead, I still see one, albeit smaller issue.

    If you are supporting code that isn’t testable, or the company development philosophy doesn’t include/allow for unit testing of code, exceptions become the only guaranteed way to enforce the fail fast principle.

    Someone might have forgotten to check the outcome of a returned Result object, causing the code that comes after, which could have side-effects, to be executed.

    To ignore an exception there at least has to be a deliberate decision to do so via try-catch blocks, rather than pure forgetfulness, which would have been caught in unit tests

    How likely is this to happen?
    Probably not a whole lot unless working with legacy code/languages.

    What are your thoughts on this, Vladimir?

    • http://enterprisecraftsmanship.com/ Vladimir Khorikov

      There are 2 things to unpack here.

      1) In C#, it’s easy to ignore the returned value so it’s indeed might become a problem. Many people prefer exceptions just for that reason alone. It would be nice if C# had a syntax similar to F# where you are forced to pipe that value somewhere. There’s even a special function for that: `getCustomer(id) |> ignore`. But I personally think that the cons of having exceptions still overweight its pros.

      2) It’s a good idea to differentiate exceptions that are used to enforce Fail Fast from exceptions that are used to control the flow. You normally don’t catch the former. They propagate through the whole execution stack and stop the current operation, that’s a good use of exceptions. This way they denote an exception situation, something you can’t recover from at runtime. Only the latter type needs to be replaced with Result.

      In my experience, tests usually help cope with the problem of forgetfulness. It’s just they need to validate the return value, not expect an exception (in case of using Result instances).

  • Joker_vD

    Hm, I think I generally agree with all of it, but… what about a code like this:

    foreach(var item in itemsToProcess) {
    try {
    processor.process(item);
    successReporter.ReportSuccess(item);
    }
    catch(Exception e) {
    failureReporter.ReportFailure(item, e);
    }
    }

    ? It goes against “you shouldn’t wrap such code with a generic exception handler” rule and pretends that it actually knows what to do with any exception thrown: report the failure to process the item, and go on, which goes against “log the exception and gracefully shut the application down”.

    The scenario “you’re given a batch of items and is expected to process all of them (baring truly catastrophic failures i.e. StackOverflow/OutOfMemory/etc.) and report the ok/err status on each item; you have no ‘try to mitigate an exception’ strategy, if an item is failed to be processed, it failed, go to the next one” is not that uncommon and yet it seems that your article prohibits one from implementing it. Does it really or did I just misread something?

    • http://enterprisecraftsmanship.com/ Vladimir Khorikov

      There are two questions here.

      It does go against the rule described in the article and it’s an anti-pattern. The exception here is used to control the program flow, not to report an unrecoverable situation.

      At the same time, you still can implement the batch processing scenario, it just requires a bit more work. Make the failure to process an item explicit by returning a Result object from processor.Process() ( http://enterprisecraftsmanship.com/2015/03/20/functional-c-handling-failures-input-errors/ ). It’s more code but the benefit would be the increased readability as you you’d see right away what’s going on in the application. For example, in your version, it’s not clear whether only one or both methods can fail, you’d need to go down and examine each method’s implementation. Whereas if you rewrite it like this:


      foreach(var item in itemsToProcess) {
      Result result = processor.process(item);
      if (result.IsFailure)
      continue;
      successReporter.ReportSuccess(item);
      }

      it becomes clear that only one of the methods is designed to fail. The other one should always do its work correctly.

      This is what I call method signature honestly, it’s one of the most important aspects of code readability and one of the tenants of functional programming: http://enterprisecraftsmanship.com/2016/04/21/what-is-functional-programming/

      • Joker_vD

        That’s completely reasonable, but making processor.Process() return Result requires me to write an adapter around existing processors — with catching exceptions and reporting them as failures, because the existing processors are not developed as a part of the application (they’re developed by others). So, what exceptions from them should I catch and report as failures? I fully expect a processor to throw all kinds exception (and they do) and decide that unless it’s a process-killing exception, my coping strategy is note it and go on with the iterating. So, I write
        try { inner.process(item); }
        catch (Exception e) { return Result.Fail(e); } // I still want to log the details
        return Result.Ok();

        Am I wrong? Should I instead nag the processors’ developers to actually switch to using Result and throw only in catastrophic/unthought of situations, so I don’t have to write the adapter? Do I ask them instead to introduce a base-class exception for “we expect you to be able to do something with this” situations (not a fan of such approach, personally), and wrap only those in Result.Fail() in my application-side adapter? Should I look at the processors’ internals, enumerate the exceptions they may throw and catch only those (a task impossible to complete)? The first alternative is attractive (make others sort out their exceptions and Results) but not practical.

        • http://enterprisecraftsmanship.com/ Vladimir Khorikov

          It’s fine that other people’s libraries throw exceptions. A situation that is exceptional in one context (the context of a particular library) can be expected (non-exceptional) in the another context (the context of your app using that library). It’s your responsibility to sort those exceptions out and decide which you can handle (and wrap those in Result instances) and which should fail fast your application.

          I understand your concern that it can be a lot of work but I disagree that it’s impossible to complete. Unless of course the library author doesn’t make put distinctions whatsoever in exceptions they throw (basically use `new Exception()`). In most cases, you have enough information to decide which exception stands for what: its type, message and the context in which it is thrown.

          Regarding enumerating all possible exceptions a library can throw – there’s no need in that. Process only exceptions you know of. All other exceptions will represent unexpected situations for you. They will (and should) fail fast the current operation. You can’t possibly be sure that you have the exhaustive list of the library’s exceptions anyway as this list may change. Deal with them as they come up.

          Also, not all exceptions should be wrapped into Result, here’s more on this:
          http://enterprisecraftsmanship.com/2017/03/13/error-handling-exception-or-result/