This is my personal blog. The views expressed on these pages are mine alone and not those of my employer.

Monday, 13 September 2010

Preventing Arrow Code

One of my pet hates in software is arrow code, that is code that contains excessive nesting of conditional statements and loops.

Take a look at this unwieldy mess:

   1:  Private Sub returnCodes()
   2:          code.Items.Clear()
   3:          If IsNothing(mycls.Dates) = False Then
   4:              If mycls.Dates.Rows.Count <> 0 Then
   5:                  Dim myrow As DataRow
   6:                  Dim myindex As Integer
   7:                  For Each myrow In mycls.Dates.Rows
   8:                      If getId(Input.Text) = Trim(myrow("id")) Then
   9:                          For myindex = 0 To code.Items.Count - 1
  10:                              If Trim(code.Items(myindex)) = Trim(myrow("year")) Then
  11:                                  Exit For
  12:                              End If
  13:                          Next
  14:                          If code.Items.Count - 1 < myindex Then
  15:                              code.Items.Add(myrow("year"))
  16:                          Else
  17:                              If String.Compare(Trim(code.Items(myindex)), Trim(myrow("year")), True) <> 0 Then
  18:                                  code.Items.Add(myrow("year"))
  19:                              End If
  20:                          End If
  21:                      End If
  22:                  Next
  23:                  code.Sorted = True
  24:                  If code.Items.Count > 0 Then setCode(code.Items(0))
  25:              End If
  26:          End If
  27:      End Sub

Pretty ugly huh? A couple of problems with this:

There are a high number of distinct paths through the code
The amount paths through a section of code is directly correlated to the chance of it containing a bug.  You can't realistically test every single possible scenario on your code - its simply not possible, therefore the only answer is to reduce the possible paths your code can take.

Excessive looping
Loops use up resources - fact.  A piece of code that loops 5 times is quicker than a piece that loops 500.  It is therefore important to use looping carefully to efficiently use resources.

Its hard to debug
Chances are you won't be the only person who will have to read and understand your code. You might understand it fully when you write it, but what about a year down the line? What about your colleagues when you aren't about?

Coding like this will only contribute to a slower, harder to debug, error prone software.

So that I could ultimately understand and subsequently modify, I had to simplify it whilst also keeping in mind the above points.


   1:  Private Sub returnCodes()
   2:          code.Items.Clear()
   4:          If IsNothing(mycls.Dates) Then Exit Sub
   6:          Dim myrow As DataRow
   8:          For Each myrow In mycls.Dates.Rows
   9:              If getId(Input.Text) = Trim(myrow("id")) Then
  10:                  If Not code.Items.Contains(myrow("year")) Then
  11:                      code.Items.Add(myrow("year"))
  12:                  End If
  13:              End If
  14:          Next
  16:          code.Sorted = True
  18:          If code.Items.Count > 0 Then setCode(code.Items(0))
  20:      End Sub

Not only is the code easy to understand, I actually increased performance dramatically.  This process reduced loops from 2480 to 509

The most important lesson I've learnt is that there is simply no excuse for any good developer to submit code without  removing the unnecessary crap.  See this article for some of the hints on cleaning up your own code.