Sometimes, you can simplify a complex class by extracting out a specialist helper class to provide dedicated assistance. And, sometimes, old lessons turn out to be the most relevant.
I was recently working on a complex processing class that formed the centre of some significant application functionality – the exact details aren’t important here. Part of this processing involved loading a large number of description instances from the database. There are too many of these descriptions in the database to load them all into memory at once, so they are loaded into memory as needed.
The original version of this code ran correctly, though relatively slowly. When the original developer investigated the reasons behind the performance problem, he found the same descriptions were being loaded from the database hundreds of times during a given processing run.
To improve performance, he added some caching. Any description, once loaded, is retained in memory until the end of processing, allowing a lot of unnecessary database access to be avoided. This directly leads to significantly better performance.
Unfortunately, caching was added to the original class by adding storage and cache checking at every point where descriptions were originally loaded from the database. Since there were a dozen or so points of access, the code to access and update the cache was repeated a dozen or so times. (Can you see what went wrong?)
Of course, the obvious problem occurred: one of the sites updating the cache had a simple error in its logic, corrupting the cache and resulting in process failure.
Given the title of this post, you can guess at the how we fixed this problem once we’d found it: We extracted a simple description cache as a separate helper class.
We made no effort to make this helper class generic and reusable - it’s explicitly there to support the main processing class and nothing more. In fact, in other circumstances, we could have made it a nested private class. Centralizing the cache logic in one place made it easier to verify and test, and allowed us to simplify the main processing class significantly.
By the time we had finished, we’d fixed the original bug, extracted a specialist helper class, improved performance still further, reduced the number of lines of code in the system, and made future maintenance of the processing class simpler because delegating caching to the helper class means it now had fewer responsibilities.
The lesson here is that we ended up with a nasty and hard to resolve bug in our system because we ignored two important principles:
-
Don’t repeat yourself (DRY)
If we had adhered to these principles up front by introducing the helper class as soon as it was clear it was needed, this particular bug would have never appeared and we would have saved a considerable amount of time.
What’s most frustrating is that these aren’t new concepts to us – they’re guiding principles we thought we knew well. One key take away is that it’s as important not to forget the old lessons we’ve already learnt as it is to learn new lessons.
Comments
blog comments powered by Disqus