Exceptions for flow control in C#

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 aim 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 isn’t 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, though. 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.

Subscribe


I don't post everything on my blog. Don't miss smaller tips and updates. Sign up to my mailing list below.

Comments


comments powered by Disqus