Written By: Steve Zagieboylo, Senior Architect at Calavista
This is the fourth part in the series, “Technical Debt Patterns.”
Tree Rings occur whenever you have wrappers around wrappers, sometimes several layers deep. Sometimes this happens because a core of important code is too complex or too fragile for anyone still around to be prepared to edit it. Maybe the core represents an API that is called by code outside.
Developers have been afraid to edit a fragile bit of code, so new functionality is made by putting a layer around it and calling in. This can happen multiple times, and you can tell the age of a library by the number of rings around it.
Unit Tests (As Always)
Analysis Step OneFirst, identify the rings, if there are two, or three. This is not as straightforward as this simple instruction makes it sound. Sometimes there are layers to an API that make perfect sense and are a good architectural choice — as Mark Richards says in Software Architecture Patterns, it is a good idea to have a clear separation between the Presentation, Business, Persistence, and Database layers in the software. You don’t want to look at these layers and declare that they are tree rings.
Where you are likely to see Tree Rings is inside the Business Layer, and then it bubbles up to the Presentation or layer. In a web application, this is the REST API layer. When you see more than one Resource file (or REST API file) that sit over a given set of entities in the Persistence layer, that’s one sign that you might have a problem. Sometimes it isn’t a separate file or set of files, just an entirely separate section within the files, with a bunch of APIs that no one really owns any more, but then a bunch of new ones that replicate or overlap significantly the old ones. Sometimes they will even call the old ones, then filter or merge or otherwise transform the results.
The other time you will see this problem is in the Persistence Layer, when too much Business logic has gotten pushed into it, or, more likely, there was not initially a clean separation of Business and Persistence, but then someone added another layer because it was too muddled.
Analysis Step Two
Now that you’ve identified where the grunge is, figure out where you want to head but don’t start those changes yet. This is where you probably need to add unit tests before you proceed. Work out what the outside world really depends on from these inner pieces, and make thorough unit tests for them. If there are other parts of your own code that have been sticking their fingers into your inner rings, then make wrappers for those inner parts, possibly in the outer part where the API should have been in the first place, and change them to call the new API.
Finally Making Changes
Finally, you can work out what the right layers are. Make sure that the business logic is no longer in the Persistence Layer, and that you have only one, cohesive business logic class for each concept. If those classes are getting too big, then you possibly want to refactor your architecture so that the concepts are more cleanly defined and delineated.
If your unit tests are comprehensive, this is something that you can do over time. But that means you’ll have to budget that time in. You can’t just do the analysis and then congratulate yourself. Pick the worst one, where there are two or even three Business Logic wrappers around some set of the persistent entities, and clean up that one mess. Then you’ll have a good idea of how much time and effort it takes, plus you have improved your average code cleanliness by quite a bit, since you took your worst section and (hopefully) made it one of your best.
2 Comments
Comments are closed.
This is a great bit of advice. Refactoring once you’ve locked down the code’s contract through unit tests is invaluable. Essentially you just need the I/O locking down through the unit tests, then you can refactor to your heart’s content. I’ve worked for an idiot before who treated refactoring as an incredibly difficult job. Like my Engineer grandfather used to say “get the hell out of my way boy!”. He also used to say “right tool for right job”. When I realised he wasn’t insulting me, suddenly an angelic quire burst into song and a golden light shone down on my face! I had a moment of epiphany! There was always a way to fix the technical debt.
However, I find contracting a real pain because of the inability to tackle anti-patterns like this sort of thing. Technical debt isn’t as tangible as a return, so you cannot justify your hours, even though you may be able to see the solution and being conscientious, you may want to fix this sort of issue. The key is to try and talk to your manager and do some unpaid overtime like I usually gift to my client in order to tackle this sort of overhead.
It’s great that someone has articulated in a clear and concise manner what many of us have advocated for years but been unable to objectively identify the warning signs. Great article Calavista!
Thanks, Chris! I really appreciate the kind words. And I’m impressed with your diligence in spending your own time to clean up your client’s code.
While Calavista is a consulting company, we do project-based consulting, not hourly work. We generally charge our customers for the whole team, including project management, on a weekly basis. Also, we manage the project, so we have the power to make the sort of trade-offs of paying down technical debt vs. implementing new features.
We are sometimes hired specifically to help manage and fix their technical debt issues, so that is the focus of our efforts. (This is what inspired me to write this series of blogs in the first place.) It is very satisfying to clean up a body of code into something that is graceful, but it is doubly satisfying when that is specifically what you’re being paid to do. 🙂
Again, thanks for the comment.
Steve Zagieboylo,
Senior Architect, Calavista