Avoid Unnecessary Braces

Nowadays, I like to avoid curly braces—{ }—unnecessary curly braces, that is.

I used to think braces everywhere were great—that they made code blocks more explicit and helped avoid bugs. I no longer believe that.

Below we have a function definition. The function cycles through a collection of Customers and determines whether any of them have current orders:

  private bool HasCurrentOrders(IEnumerable<Customer> customers)
  {
     foreach (var customer in customers)
     {
        if (customer.HasCurrentOrders)
        {
           return true;
        }
     }
     return false;
  }

There are four lines of code—a foreach, an if statement and two returns.

Optional braces consume another four lines. Here is the function without those braces:

  private bool HasCurrentOrders(IEnumerable<Customer> customers)
  {
     foreach (var customer in customers)
        if (customer.HasCurrentOrders)
           return true;
     return false;
  }

There is a good chance you don’t like this shortened version. I understand. I was in the same place.

I feel that the braces were taking away from the readability of the code. Without braces, the function is easier to understand. Indeed, when you have a 200 line function (or even a 20 line function), braces are an essential scoping tool. However, dropping the embellishing braces is as easy as pie when there are only a few lines of code.

So, what if we need braces because we have multiple statements to execute in a conditional block?

In this situation, I like to extract the conditional logic inside an if or else block into well-named helper methods.

For example,

  public void SaveCustomer(Customer customer)
  {
     if (customer.IsNew)
     {
        ValidateCustomerForAdd(customer);
        DataStore.Add(customer); 
     }
     else
     {
        ValidateCustomerForUpdate(customer);
        var existingCustomer = DataStore.Get(customer.EmailAddress);
        var existingCustomer.Update(customer);
        DataStore.Update(existingCustomer); 
     }
  }

simplifies to

  public void SaveCustomer(Customer customer)
  {
     if (customer.IsNew)
        AddCustomerToDataStore(customer);
     else
        UpdateCustomerInDataStore(customer);
  }

Wow! That is so clear and reads like natural language.

We also don’t need braces when we throw exceptions:

  protected virtual async Task<AccountGroup<Account>> GetAccountGroup(string accountGroupName)
  {
     var accountGroup = await AccountProvider.GetAccountGroupByName(accountGroupName);
     if (accountGroup == null)
        throw new InvalidAccountGroupName(accountGroupName);
     return accountGroup;
  }

or when we return values:

  public string BuildCustomerNumber(Customer customer)
  {
     if (customer.IsSourcedFromAffiliate)
        return BuildAffiliateCustomerNumber(customer);
     else
        return BuildRegularCustomerNumber(customer);
  }

One of the frequent objections to dropping unnecessary brackets I hear is that it will lead to bugs when adding more logic to a single line conditional block.

To demonstrate with our BuildCustomerNumber() function, let’s say that when we build an affiliate customer number, we must first perform some validation, and we leave out the braces:

  public string BuildCustomerNumber(Customer customer)
  {
     if (customer.IsSourcedFromAffiliate)
        ValidateAffiliate(customer); 
        return BuildAffiliateCustomerNumber(customer);
     else
        return BuildRegularCustomerNumber(customer);
  }

Well, this will cause trouble; when the predicate if statement evaluates to true, only the ValidateAffiliate() function is deemed to be inside the true conditional block. That means the first return statement is no longer governed by the conditional and will now always be executed—our changes have made the else block redundant—and we have introduced a bug! 

However, we have a suite of unit tests that will pick up the broken code.

You always write unit tests to guarantee the expected behaviour, don’t you?

And the solution to including validation as part of the true branch is as simple as it is obvious: Push the call to ValidateAffiliate() down into BuildAffiliateCustomerNumber(). 

And we are done.

In conclusion, I highly recommend increasing code readability by avoiding unnecessary braces. And, if you’re not doing this already, write unit tests to ensure your code works as it should!