Top 10 Dotnet Exception Anti-Patterns in C#

As a programming instructor and former software engineer / software engineering manager I have seen a lot of mistakes around exception management in dotnet code.

In this article we’ll explore the top 10 mistakes I see new and seasoned developers make with exception management in dotnet source code. These principles should be valid for all dotnet languages, but examples will be provided in C#.

Also note that some of these are so common you’ll also see them on my list of common dotnet gotchas.

#1: Not Catching Exceptions

The first major mistake I see people make is not accounting for exceptions that may occur in their code.

While there are plenty of exceptions that are the fault of the developer (NullReferenceException, IndexOutOfRangeException, ArgumentException, InvalidOperationException, etc.), there are exceptions that no developer can prevent from occurring.

For example, when working with files, it’s possible that the needed file does not exist, the user doesn’t have permission to open it, the file is corrupt or locked, or the hard drive runs out of disk space (among other potential problems). All of these issues can result in exceptions that would be unrealistically hard for developers to prevent from occurring.

Other common things that might fail include:

• Connecting to external APIs
• Connecting to databases
• Executing SQL statements
• Working with the operating system
• Requesting resources over the internet
• Working with hardware devices such as cameras or microphones
• Relying on an active internet connection

All of these scenarios are things that the programmer should account for by adding a catch clause for a specific exception that may occur.

Instead of:

string text = File.ReadAllText("MyFile.txt");

// Do something with text

Catch Exceptions you expect and can recover from:

try
{
   string text = File.ReadAllText("MyFile.txt");
   // Do something with text
}
catch (IOException ex)
{
   // Display an error message
}

This way your code works correctly not just for the expected scenarios, but also can gracefully recover from and communicate problems to the end user.

Catching expected exceptions can make the difference between an informed and happy user and a crashing application the user never wants to touch again.

#2: Not Throwing Exceptions

While not catching exceptions can lead to a brittle app, not throwing exceptions can also lead to an influx of errors.

If you encounter a scenario where your code cannot reliably achieve a good result, throwing an exception can help you discover this bad scenario immediately instead of having to wade through the results of your code’s decisions later on.

For example, consider a FindLargestNumber method that takes in an array of ints and returns the largest one.

One possible implementation of this follows:

public int FindLargestNumber(List<int> numbers)
{
   int largest = int.MinValue;
   if (numbers != null)
   {
      foreach (int num in numbers)
      {
         if (largest < num)
         {
            largest = num;
         }
      }
   }
   return largest;
}

Never mind that there are more efficient ways of doing this (including LINQ’s Max method). Let’s look at some flaws here:

• If numbers is null, int.MinValue will be returned
• If numbers is an empty list, int.MinValue will be returned

While I don’t see a way of making this code actually error, it may not make sense to the caller that the method returns -2.1 billion if you pass it a null or empty list.

This -2.1 billion value, if not noticed, could get persisted to a database, included in a report, or used for calculations. This would result in some values that are definitely not what you might expect.

This is a bit of an extreme example, but it’s not too hard to see how a small decision of returning a sentinel value could cause bugs much farther downstream. These bugs would take some time to diagnose and identify where the -2.1 billion came from, which subtracts from your team’s total productivity.

In these cases, it might make more sense to throw an exception and let the user know they tried to use your code in a way that is not supported. This increases the odds of strange behaviors being found at development time instead of during testing or production use later on.

#3: Catching the Wrong Exception Type

Another problem I see from new developers in particular is that they catch the wrong exception type.

New developers will sometimes catch exceptions that should be avoidable such as NullReferenceException or IndexOutOfRangeException. For example, in the code below, the programmer has a bad for loop and has chosen to catch IndexOutOfRangeException instead of fixing their code:

int sum = 0;
try
{
   for (int i = 0; i <= myArray.Length; i++)
   {
      sum += myArray[i];
   }
}
catch (IndexOutOfRangeException ex)
{
   // Do nothing
}

Console.WriteLine("The total is " + sum);

The code above will always encounter an IndexOutOfRangeException, and this will cause it to hit the catch block and then recover to print the correct total.

While this code generates a good result, the try / catch is avoidable here and the presence of the exception being thrown slows down the application significantly.

Instead, the developer should have fixed their code by using < instead of <= in their for loop.

---

Another mistake I see developers make in catching exceptions is to catch Exception either with an explicit statement like this:

try
{
   // Some code
}
catch (Exception ex)
{
}

Or with the newer syntax of:

try
{
   // Some code
}
catch
{
}

Both of these statements catch Exception or anything that inherits from it. This means that if anything goes wrong in either of these blocks it will enter the catch block.

This may sound like a good thing, but keep this rule in mind: only catch exceptions that you can recover from.

By catching any possible exception in a block, you are stating that your app can gracefully handle it. This could be a security exception, something related to IO or database access, or it could potentially be a critical system issue such as an out of memory issue or critically low disk space. Even worse: catch Exception can hide programming mistakes such as null references and index out of range exceptions, resulting in code that never works but doesn’t break enough to be noticed.

To a new developer, this doesn’t sound like something that might actually happen. However, I can tell you from experience that I have inherited several different codebases at multiple companies that hid years-old errors in code by catching exception. What’s worse: I only found this out after hours of debugging a related symptom caused by this old behavior.

Do yourself a favor: don’t catch exception. Only catch exceptions that you can handle at the spot you’re catching them.

#4: Throwing Exception

If catching a generic Exception is bad, then it should make sense that throwing a generic Exception is bad as well.

Let’s say you validate values with the following C# code:

if (quantity <= 0)
{
   throw new Exception("Quantity must be a positive number");
}

This code does detect problems early and does surface those issues in an Exception. That’s good.

Unfortunately, this code doesn’t throw a specific exception such as an ArgumentOutOfRangeException or an InvalidOperationException. Instead, this code throws an instance of the Exception class.

This means that any code that wants to catch this exception must catch Exception. This forces that code to also catch any other exception that might have been thrown, even if that code is not able to successfully recover from those types of exceptions.

Instead of throwing a new Exception, the code above should have thrown a more specific exception that can be specifically caught and handled by a catch block.

#5: Incorrectly Rethrowing Exceptions

Here’s another quick one I often see.

Let’s look at some exception management code that tries to re-throw an exception object:

try
{
   // Do something that might cause an exception
}
catch (IOException ex)
{
   // We decide we don't really want to handle this exception
   // Let's re-throw it so calling methods can catch it
   throw ex;
}

Here this code looks like it ought to be correct. However, the throw ex syntax above is a bad practice.

What the author of this code is trying to do is to let the exception bubble up the call stack and be caught by other code. This code does do this.

However, this code also removes any previous context we had in ex as to where the exception occurred. The call stack of the exception will start inside the catch block, not where the exception originally occurred.

Instead, we should do the following:

try
{
   // Do something that might cause an exception
}
catch (IOException ex)
{
   // Rethrow the exception without replacing its call stack
   throw;
}

This causes the exception to propagate upwards without replacing its call stack. This will preserve full details of the exception for the code that ultimately handles it.

Personally, I wish this distinction was not necessary, but this is the behavior we have with throw for rethrowing exceptions, so it’s good to be aware of it.

#6: Catching Exceptions in the Wrong Place

One of my cardinal rules for catching exceptions is to only catch exceptions in places where you can correctly recover from them.

This means that just because you know a line of code may throw an exception doesn’t mean you should have a try / catch around it.

Instead, you may want the code that calls the method you’re in to have the try / catch if it is better equipped to react to exceptional circumstances.

Ultimately something should handle an exception that may occur, but you as a developer must decide what the most appropriate place for handling that exception might be.

#7: Using Exceptions for Flow Control

One of the things new developers hear a lot is the following mantra:

Never use exceptions for flow control!
-Most Computer Science teachers and senior developers

However, we rarely explain exactly what that means (and even if we do, it’s hard to internalize).

What it ultimately boils down to is this: exceptions should be truly exceptional.

An exception is something that’s truly unusual and may occur only in rare circumstances, not a normal part of program execution.

Good examples of using exceptions:

• A class requires a parameter that has a valid value and was given an invalid one
• The system needs to talk to the database, but the database appears to be offline
• The file the user is trying to load is not in the correct format

Bad examples of using exceptions:

• We ran out of items to process in a list
• The user chose quit on the main menu
• In a guess the number app, the user guessed the correct number

Exceptions are good ways of responding to exceptional circumstances, but if you can handle things using an if statement or proper loop, do that instead.

Why?

Because exceptions are harder to think about and it’s harder to envision their flow through an application. Additionally, throwing an exception takes a bit of time because dotnet needs to generate a valid stack trace and other diagnostic information and this takes some time.

#8: Not Using Custom Exceptions

Dotnet offers a wide array of exception types suitable for throwing in a variety of situations including:

• ArgumentException
• InvalidOperationException
• FormatException

However, there are some times when you need to throw an exception and none of the built-in types make sense.

In these cases it can make sense to create a new class representing your custom exception and inherit from Exception.

For brevity, I’ll not detail the process for this in this article, but expect some additional content from me in the weeks ahead on creating custom exceptions in C#.

One added benefit of creating a custom exception type is that you can catch these exceptions without fear that you might be catching anything already built into dotnet.

#9: Not Providing Enough Exception Details

It’s not enough just to throw an exception in most cases. When exceptions are caught you need to be able to react to them.

Reacting to exceptions typically requires:

• Knowing where the exception occurred (the stack trace)
• Knowing the exact type of the exception (the GetType method helps with this)
• Having a helpful message for the exception
• Having additional details specific to the exception that occurred

The following code is an example of not providing enough information when throwing an exception:

public void WithdrawMoney(decimal amount)
{
   if (amount <= 0)
   {
      throw new InvalidOperationException();
   }
   // Actual withdraw code
}

This code successfully handles cases where the amount is not positive, but any code that catches it does not have the relevant details to display information to the end user or log an appropriate message in the error log.

A better implementation would be:

public void WithdrawMoney(decimal amount)
{
   if (amount <= 0)
   {
      throw new ArgumentOutOfRangeException(nameof(amount), amount, "The amount to withdraw must be positive");
   }
   // Actual withdraw code
}

In this case we’re providing the name of the parameter that is invalid, the value of that parameter, and a helpful error message.

#10: Not Trusting the Framework

I’m going to say something nearly heretical: You don’t always need to handle exceptions.

Sometimes, when building an application in a larger framework, it can make sense to not catch exceptions yourself. In these cases you allow your exceptions to bubble up and be handled by the framework.

For example, when developing a dotnet Web API using ASP .NET, it can make sense to not worry about catching exceptions yourself. In these cases it can make sense to let ASP .NET (or another framework) to catch the exception and log and handle it using the default mechanisms.

This won’t work for every framework you use, but if your framework of choice has built-in mechanisms for handling exceptions, it can make a lot of sense to rely on those.

Key Points around C# Exceptions

These are my top 10 guidelines around exception handling in dotnet and C#.

Some of these may appear to be contradictory at times, but the rules really boil down to these principles:

• Exceptions should be reserved for the truly exceptional
• Exceptions should include all information needed to react to them
• Exceptions should only be caught where they can be appropriately responded to
• Do not catch Exception or force others to catch it by throwing Exception

Exceptions add to your cognitive workload as a developer and slow down your application and should be avoided when possible
You may not agree with all of these. I know of many developers who try to avoid exceptions wherever possible and rely instead on functional programming techniques. This is fine, and I agree with many of these practices, but this article exists to serve those dealing with traditional exception management.

You also may have other rules I’ve not considered. If that’s the case, I’d love to hear your best practices and lessons learned.

Comments

[Krirk Srithaweewath]

11 . Catch the exception then throw a cutomer exception without the information from actual exception.
In this case the caller cannot get any information from ex;
It can be anything it might not relate to reading the file.

try{

    /*
    Code to read file and do something here.
    */

} catch (Exception exThatNeverBeUsed){
    throw new Exception("The file does not exist");
}

[Matt Eland]

Oh there's just so much wrong with this... and yet it happens all the time.

[Prashant Choudhary]

First point is misleading, I know you add details in point 6, but still. Catching the exception just because it thrown doesnt make sense. If A FileNotFound cant be recovered then what else can you do? Let it just throw. Catching and not doing anything is just going to hide the error.

[Zhang Li]

On point 2, can you explain how the LINQ .Max is more efficient?

[Matt Eland]

For one, it's less code. Since you can call Max on a collection, you don't need to iterate over that collection by yourself.

For another factor, LINQ's Max function is an extension method implemented as follows:

        public static int Max(this IEnumerable<int> source)
        {
            if (source == null)
            {
                ThrowHelper.ThrowArgumentNullException(ExceptionArgument.source);
            }

            int value;
            using (IEnumerator<int> e = source.GetEnumerator())
            {
                if (!e.MoveNext())
                {
                    ThrowHelper.ThrowNoElementsException();
                }

                value = e.Current;
                while (e.MoveNext())
                {
                    int x = e.Current;
                    if (x > value)
                    {
                        value = x;
                    }
                }
            }

            return value;
        }

While this isn't doing as much heavy lifting under the hood as I might have expected (though it does explicity bypass foreach), it's entirely possible that future versions of Max might see additional performance improvements that we don't need to worry about implementing ourselves.

So, less code for us to write and it takes advantage of current and future performance optimizations in LINQ.