Refactoring method with nested loops and 6 variables

Question

I have a method which implements a small section of a (complex) algorithm. This algorithm has taken me several weeks to perfect and is by far the fastest method I can find. The method in question has 3 levels of nested loops and each run of the innermost loop refines the values of 6 variables which in turn determine the behaviour of the other loops.

I therefore have a method with nested loops and ifs with 6 variables that are frequently read from and assigned to. I can understand it perfectly, I wrote the thing, but I fear that it may be a bit daunting for someone who doesn't have me there to explain it.

I know I have too many loops for the code to be easily readable:

I am looking for tips on how to go about refactoring this and other similar methods to make them more readable.

For scale the method is currently ~70 lines long.

Answer 1

Without seeing the code, or insight into what language this is implemented in its tough to provide a problem-specific solution. You say method so that may exclude some solutions that would be applicable in certain languages.

From the way the solution is posed it seems that speed is of the essence but you are bound to incur some overhead by decomposing the loop, although (if you are using a compiled language) it may inline some things for you.

If you are using C, C++ or another language that has pointers I would consider decomposing the loops into appropriately named functions and pass along those variables that the loop requires.

Additionally, depending on how your computation is laid out in memory you may still retain the speed you have acquired if the appropriate pointers are passed as you should not incur any cache-misses that you did not already have. But keep in mind conditional branching as you progress.

If you are using another language, you may have success in applying some of Martin Fowler's teachings (http://martinfowler.com/articles/refactoring-pipelines.html, http://www.refactoring.com/catalog/splitLoop.html).

The split loop is an often used performance optimisation in data intensive applications. When you are accessing to separate arrays in the same loop you can get hit badly by the cache misses. That is, if the arrays are large enough you loose locality of reference and the cache fails to do the job. Put enough code here and every operation will not hit the cache and instead have to go back out to main memory.

By splitting loops up into small separate components that only act on one array you get significant performance increases. In rendering code I've seen up to an order of magnitude difference. This is particularly beneficial if it is primitive type based and less so if class reference or pointer based where you need an additional dereference off into a random memory location. This technique is also very similar to loop unravelling for performance gains (although something I doubt would ever appear in a refactoring/patterns/OO type book :)

The last suggestion is attempting to simplify your expression using logic or determining if there is some data-structure that would better suit your computation in the legibility aspect.

Answer 2

If there are a series of complex checks or calculations chained together by logic then give each a descriptive name so that the purpose is clear.

So:

Private Function CheckDateEntry(int day, int month, int year)
  If DayIsValid(day) And MonthIsValid(month) And YearIsValid(year)
    Return True
  End If 
End Function

Rather than:

Private Function CheckUserInput(int d, int m, int y)
  If (d > 0 and d <= 31) and (m > 0 and m <= 12) and (y > 0 and y <= 2015)
    Return True
  End If
End Function

If you can, break the routine up into smaller (testable) routines. Developers have a lot more faith in a piece of code if there are a suite of automated tests for it.

Make sure your routines and variables following a sensible convention. So verb-noun for methods and noun for variables.

In your quest to make the code as fast as possible, it sounds like some code readability has had to be sacrificed, which is par for the course sometimes. However, if the code is unclear as a result of this in places, consider adding pertinent comments.