In my current job, I’ve been tasked with cleaning up old code a few times. Often the code is a labyrinth and the data behind it is even more tangled. I find myself combing out things into nice, neat, modular methods. Each method does one thing and does it well. That’s when things start to go south…
Invariably, I end up with a clean API and no real way to tie it all together. The solution has been to write a big ugly “glue” method (generally full of conditional statements) that eventually calls all of my “clean” methods.
The glue method usually ends up being a terse version of the tangle of code/data that I was attempting to clean up. It’s generally more readable, but it’s still annoying.
How can I avoid such methods? Is this a symptom of the tangled data or a reflection of something that I’m doing wrong?
6
I will give you our experience refactoring LedgerSMB. We made a decision to do things differently early on and are still doing exactly what you describe but without a lot of glue methods (we have a few glue methods btw, just not a lot).
Life with Two Codebases
LedgerSMB has survived with two codebases for about 5 years and it will be several more before the old codebase is eliminated. The old codebase is a true horror to behold. Bad db design, Perl constructs like IS->some_func(%$some_object); along with code which shows exactly why the spaghetti metaphor is sometimes used (execution paths meandering between modules and back, and between languages, with no rhyme or reason). The new codebase avoids this by moving db queries into stored procedures, having a cleaner framework for request handling, and much more.
The first thing we decided to do was to try to refactor module by module. This means moving all functionality in a specific area into a new module and then hooking the old code into the new module. If the new API is clean, this isn’t a big deal. If the new API is not things get hairy and that’s an invitation to work a little harder at the new API….
The second thing is that there are plenty of times when new code has to access logic in old code. This is to be avoided to the extent possible because it leads to glue methods that are ugly but one can’t always avoid it. In this case the glue methods should be minimized and avoided to the extent possible but used when necessary.
To make this work you have to commit to rewriting all functionality in a specific area. If you can, for example, rewrite all customer information tracking code at once, that means that the code which calls this from the old code is not hard to work with, and dispatching to the old code from the new code is minimized.
The second thing is that if you have reasonable abstractions in your place, you should be able to choose which level of the API to call and how to keep that clean. However, you should think about rewriting the portions that are calling your API so that they are quite a bit cleaner as well.
There are many areas of business tools that are irreducibly complex. You can’t get rid of all complexity. But you can manage it by focusing on clean API’s which specifically do what you need to do, and modules which utilize that API constructively. Glue should be a last resort only after considering that rewriting the rest of the calling code may be faster.
2
It sounds like what you’ve done is taken a tangled mess of a precedural codebase and created a lovely modular precedural codebase.
Invariably, I end up with a clean API and no real way to tie it all together. The solution has been to write a big ugly “glue” method (generally full of conditional statements) that eventually calls all of my “clean” methods.
With procedural code (even if it’s disguised as OO), you’re always going to end up with some kind of sequential workflow defined somewhere, often filled with complex conditional branches as you describe. I suspect it’s this procedural nature of the code which is making you feel that something is wrong. This is not necessarily a bad thing, and when working with legacy code may be entirely unavoidable
0
You should clean up the big ugly glue method the same way you cleaned up the original code base. Split it up in neat modular methods. You probably have groups of lines of code that do some task split these lines up in methods, if you share some variables you could consider putting the shared variables and the new methods in one class.
2
Basically, you keep adding abstraction layers, until it looks right at each layer taken on its own. The paradoxical thing about abstraction is you add complexity to reduce it, because when you read abstracted code, you only concern yourself with one layer at a time. If each layer is small enough to be easily understood, it doesn’t matter how many layers it’s resting on.
That’s also what makes abstractions difficult to write. Even something as simple as a pencil is mind bending if you try to hold all its layers of abstraction in your head at once. The key is to get one layer the way you like, which you have done, then forget all the complexity that underlies that layer and do the same thing on the next level.
It sounds like you are refactoring the API by just thinking about the implementation of the API, but without thinking enough about the code that uses the API – that is, the “glue code” you are talking about.
If that’s true you might try to start at the other end. Rewrite the stuff that threatens to become your ugly glue code first, and create a couple of not yet implemented interfaces that will become your API in that process. Do not think too much about the actual implementation of this API yet – it’s OK if you have the gut feeling that you can do it. And only then rewrite the code labyrinth to conform to that API. Of course there will be some changes in the API and the glue code in this process, but it should fit together better.