Say "no" to silent failures in your methods
Tags: programming, exceptions
Published: 2022-05-18 18:48:41

One of my personal pet peeves when working on code is encountering logic that checks for some unexpected state and when that unexpected situation applies... does nothing. This is a classic case of sweeping mistakes under the carpet. Consider the following C# code:

{% gist https://gist.github.com/bhaeussermann/964d83dd78431027155f175be2d504f1 %}

In this example we check the state of the order and if it is not in the expected state for processing, we do nothing. The problem here is that if the `ProcessOrder()` method gets called on an order that is not in the right state for processing, this is most certainly a bug in the code that is calling the method. By ignoring the unexpected state, the `ProcessOrder()` method is hiding the issue, making the underlying bug much harder to locate. 

This is because the code calling `ProcessOrder()` will assume that after the method had been called, the order had been successfully processed. The if-condition in `ProcessOrder()` makes it so that this assumption is not true so that the calling code might proceed interacting with an unprocessed order _as if_ it were a processed order. This would cause potential havoc, diverting attention away from the source of the problem.

The core problem with `ProcessOrder()` is that there are situations where the method would successfully return despite not having done what it was expected to do, i.e. processing the order.

The Solution
--------

My personal rule of thumb for preventing this kind of situation is that a method should always do what it claims to do in its name (in this case processing the order), or otherwise throw an exception. If the method returns without an exception, this _must_ imply that the operation was completed successfully. This is the central point of this post, so I will state it explicitly:

> A method must return without throwing an exception if _and only if_ it successfully did what it claims to do in its name. In any other case an exception must be thrown to indicate that what the method was expected to do could not be done.

Therefore a better implementation of the method above should look like:

{% gist https://gist.github.com/bhaeussermann/c9477764e3d7861857f0229b69125a78 %}

The Try-Parse Pattern
-------

There can be operations where it is not unusual that the operation can't be performed for one reason or another. That is, the reason for the operation not being able to be performed is _not_ due to an error or a programming bug, but merely a valid possible scenario. Since exceptions are supposed to be used for _exceptional_ cases only, such a situation calls for a different approach. Consider the following method signature:

{% gist https://gist.github.com/bhaeussermann/1fe8f51536a768becb5a7f9d7d22c897 %}

The caller should expect that if the method successfully returns, it would provide a `Route` leading from the provided source to the destination. If there is no such route, then based on the method's name, it should throw an exception because it wasn't able to "calculate the route". But what if the non-existence of a route is a common and valid use-case? We could make the method return `null` if no route could be calculated, but this would amount to a false impression from the perspective of the caller that the route was calculated when that's not the case.

This kind of scenario is often a good candidate for the [try-parse pattern](https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/exceptions-and-performance#try-parse-pattern). Based on this pattern the method signature above would change into:

{% gist https://gist.github.com/bhaeussermann/8b131fc3d436f8b5106f61797985474f %}

Now, if no route is found, the method would return `false` to indicate the fact. When it does, it's not "lying" about having performed its task because it did, in fact, _try_ to calculate the route like its name states.

The [Microsoft documentation about the try-parse pattern](https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/exceptions-and-performance#try-parse-pattern) presents this as a much more performant alternative to throwing exceptions where exceptions would be thrown for common scenarios. Another benefit of using this pattern is that it makes it clear to the method caller that a certain case needs to be handled explicitly.

Making a habit of ensuring all methods always do what their names say (or throwing an exception otherwise), makes it easier to reason about the code and helps make the source of programming bugs be more obvious when they occur.