Friday, 18 December 2009

Maintainable code and Boolean logic

After months of doing BI-related stuff, I’ve found myself doing ‘proper’ coding again – specifically Windows Forms based work in C# working against a WCF data source. Other than the realisation that Forms seems primitive compared to WPF, I’ve found myself referring back to Juval Lowy’s excellent tome “Programming .NET Components” which includes the IDesign C# coding standard. This is the definitive .NET coding standard and one I thoroughly recommend adopting (if you haven’t done so already), however it does contain some guidance whose value cannot be fully appreciated until one starts to follow it. In particular I’m thinking of rule 30 in the “Coding Practices” section:

“Avoid function calls in Boolean conditional statements. Assign into local variables and check on them”

Among other things this makes it much easier this makes to debug nested conditional statements, but it’s also helped me to realise the value of using Boolean functions. By factoring out as much code as possible to such functions, code becomes self-descriptive. To illustrate this point, look at the following code:

if (
      !(lastRowRequested <= (m_CentrePoint + (m_ToleranceWindow / 2))
      &&
      lastRowRequested >= (m_CentrePoint + (m_ToleranceWindow / 2)))
  )
{
  if (
          lastRowRequested > GetAbsoluteHalfwayPoint()
          &&
          (m_Cache.Count + m_FirstRowAbsoluteIndex)!=m_TotalCount
      )
  {
      // Do something
  }
  elseif
      (
          lastRowRequested < GetAbsoluteHalfwayPoint()
          &&
          m_FirstRowAbsoluteIndex != 0
      )
  {
      // Do something
  }
  else
  {
      // Do something else
  }
}

It’s part of a cache manager component that keeps a ‘window’ in memory on a much larger dataset. Knowing that, does the above code make sense? It certainly doesn’t to me and I wrote it! At this point I refer to rule 8 of “Coding Practices”:

“Avoid comments that explain the obvious. Code should be self-explanatory. Good code with readable variable and method names should not require comments.”

Now this is one of the many potentially controversial rules in the standard, but only because some people take it to mean “don’t write comments”. What it is actually saying is “don’t state the bleedin’ obvious”. How can we rewrite the above code to give it context and clarity? Something like this:

bool isInToleranceWindow = IsInToleranceWindow(lastRowRequested);

if (!isInToleranceWindow)
{
  bool isInBottomHalfOfCache = IsInBottomHalfOfCache(lastRowRequested);
  bool isInTopHalfOfCache = IsInTopHalfOfCache(lastRowRequested);
  bool isFirstRowOfDatasetInCache = IsFirstRowOfDatasetInCache;
  bool isLastRowOfDatasetInCache = IsLastRowOfDatasetInCache;

  if (isInBottomHalfOfCache && !isLastRowOfDatasetInCache)
  {
      // Do something
  }
  elseif (isInTopHalfOfCache  && !isFirstRowOfDatasetInCache )
  {
      // Do something
  }
  else
  {
      // Do something else
  }
}

Now the beauty of this will be evident to any BI practitioners out there, in that it gives a high-level overview of what’s going on but (thanks to the F12 key in Visual Studio) one can drill down to a lower level of detail if required (i.e. look at the function definitions). I realise that this is a trivial example, but it does demonstrate how refactoring code can make it easier to understand (and maintain).

Incidentally, as an antithesis to coding standards there is always the obfuscator’s bible, whose adherents I have to thank for providing me with a good living for several years.

No comments:

Post a Comment