In the Defence of Performance
Defensive programming is a solid practice to adopt and I generally advocate that we do so but we have to be careful we are still getting the value we expect from the practice.
Checking input for validity before we execute logic is common and aims to error out quickly before expending resources on something that will fail. In most cases this is the right thing to do.
public async Task SetAccountStatus(string accountNumber, Status status)
{
if (string.IsNullOrWhiteSpace(accountNumber))
{
throw new Exception("Invalid input");
}
await _accountRepository.UpdateStatus(accountNumber, status);
}
We’ve all seen (written) this sort of logic and it makes sense. Why would we go all the way to the database and generate an exception when we can quickly check if the account number is useful first? Golden.
Now let’s consider that the account needs to exist before we can update its status. Simples! We can just add an exists check before we try to update the status.
public async Task SetAccountStatus(string accountNumber, Status status)
{
if (string.IsNullOrWhiteSpace(accountNumber))
{
throw new Exception("Invalid input");
}
if (await _accountRepository.GetAccountById(accountNumber) is null)
{
throw new Exception("Account not found");
}
await _accountRepository.UpdateStatus(accountNumber, status);
}
Great! Except, given a missing account is an error conditon, we can assume that the majority of requests are for account numbers that exist in the database. So we’re now making two calls to the database in the majority of our requests in order to protect against a small minority of bad ones.
If we’re trying to operate on an object in a database (blob store, api, etc) and it doesn’t exist we will generally get a recognisable error that we can deal with. It makes more sense in this scenario to let the invalid request go all the way to the database and catch the resulting error. Over the majority of requests this will be far more efficient. The following code behaves exactly the same as the previous example from the caller’s perspective so is no less defensive yet orders of magnitude more performant.
public async Task SetAccountStatus(string accountNumber, Status status)
{
if (string.IsNullOrWhiteSpace(accountNumber))
{
throw new Exception("Invalid input");
}
try
{
await _accountRepository.UpdateStatus(accountNumber, status);
}
catch (NotFoundException sqlEx)
{
throw new Exception("Account not found");
}
}
Validate the in memory stuff and break out early. Anything you need to do IO for leave it alone and handle the error as this will always be more efficient under the assumption that the majority of your requests do not induce an error condition.