C# Language Exception Handling Exception Anti-patterns
Example
Swallowing Exceptions
One should always re-throw exception in the following way:
try
{
...
}
catch (Exception ex)
{
...
throw;
}
Re-throwing an exception like below will obfuscate the original exception and will lose the original stack trace. One should never do this! The stack trace prior to the catch and rethrow will be lost.
try
{
...
}
catch (Exception ex)
{
...
throw ex;
}
Baseball Exception Handling
One should not use exceptions as a substitute for normal flow control constructs like if-then statements and while loops. This anti-pattern is sometimes called Baseball Exception Handling.
Here is an example of the anti-pattern:
try
{
while (AccountManager.HasMoreAccounts())
{
account = AccountManager.GetNextAccount();
if (account.Name == userName)
{
//We found it
throw new AccountFoundException(account);
}
}
}
catch (AccountFoundException found)
{
Console.Write("Here are your account details: " + found.Account.Details.ToString());
}
Here is a better way to do it:
Account found = null;
while (AccountManager.HasMoreAccounts() && (found==null))
{
account = AccountManager.GetNextAccount();
if (account.Name == userName)
{
//We found it
found = account;
}
}
Console.Write("Here are your account details: " + found.Details.ToString());
catch (Exception)
There are almost no (some say none!) reasons to catch the generic exception type in your code. You should catch only the exception types you expect to happen, because you hide bugs in your code otherwise.
try
{
var f = File.Open(myfile);
// do something
}
catch (Exception x)
{
// Assume file not found
Console.Write("Could not open file");
// but maybe the error was a NullReferenceException because of a bug in the file handling code?
}
Better do:
try
{
var f = File.Open(myfile);
// do something which should normally not throw exceptions
}
catch (IOException)
{
Console.Write("File not found");
}
// Unfortunatelly, this one does not derive from the above, so declare separatelly
catch (UnauthorizedAccessException)
{
Console.Write("Insufficient rights");
}
If any other exception happens, we purposedly let the application crash, so it directly steps in the debugger and we can fix the problem. We mustn't ship a program where any other exceptions than these happen anyway, so it's not a problem to have a crash.
The following is a bad example, too, because it uses exceptions to work around a programming error. That's not what they're designed for.
public void DoSomething(String s)
{
if (s == null)
throw new ArgumentNullException(nameof(s));
// Implementation goes here
}
try
{
DoSomething(myString);
}
catch(ArgumentNullException x)
{
// if this happens, we have a programming error and we should check
// why myString was null in the first place.
}
