Tell The Truth

It’s really easy to lie in our code. To spread misinformation; to say things that are just not so.

Not telling the truth in code, inadvertently or not, IMHO is one of the most severe sins a developer can commit. We are going to confuse the next programmer. We’re going to waste their precious time.

The code itself always tells the truth. However, we can mislead with wrong naming of variables, functions and modules. Comments can be wildly out of date. As developers, we control programming structures that are not part of the execution but exist solely as a means of communicating our intent to other developers. It is in this domain where we can deceive.

OK, let’s look at a few instances where not everything is as it seems:

  private void InsertCustomer(Customer customer)
  {
     using (var db = new MyContextDB())
     {
        var dbCustomer = db.Customers.SingleOrDefault(c => c.CustomerNumber == customer.Number);
        if (dbCustomer != null)
        {
           dbCustomer.FirstName = customer.FirstName;
           dbCustomer.LastName = customer.LastName;
           // ... 
           db.SaveChanges();
        }
     }
  }

Spotted the problem? This method is called InsertCustomer, but in reality, it’s updating a customer! Time to rename.

How about this one:

  private decimal CalcTotalOrders(IEnumerable<Customer> customers)
  {
     decimal totalOrders = 0m;
     foreach (var supplier in customers)
        foreach (var order in supplier.Orders)
           if (!order.IsComplete)
              totalOrders += order.Total;
     return totalOrders;
  }

Spotted the misinformation? The iteration variable in the outer loop is supplier even though we are iterating over the customers collection. Maybe it’s a Copy and Paste omission? Who knows. And it doesn’t even matter. What matters is that we fix it. 

How about this method signature:

  private string FormatAge(int age, string middleName)

Why the middleName parameter? What does that have to do with the formatting of age? What else is this function doing??

Or:

  private Order GetOrder(string orderNumber)
  {
     var order = SqlDataAccess.GetOrder(orderNumber);
     var suppliers = new List<Supplier>();
     foreach(var product in order.Products)
     {
        var supplier = SqlDataAccess.GetPreferredSupplier(product);
        if (!suppliers.Contains(supplier))
           suppliers.Add(supplier);
        // Try getting suppliers from cache first before 
        // going to the database since it's so SLOW. 
     }
     order.Suppliers = suppliers;
     return order;
  }

Spotted the lie? The comment is the problem; where is the call to cache? Maybe somewhere else but not in here. Is that an obsolete or a TODO comment? Someone should find out before it causes further confusion.

So, please tell the truth in your programming. And when you do come across lies, please pay it forward by fixing them.