21

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?

cmhobbs
  • 321
  • 1
  • 6
  • 3
    APIs are meant to be used. From the tangled mess you got, you created an API and then re-tangled it. Maybe it's just the business requirement. But you added value because someone else can come and make another glue function easily using your API. No need to wring hands... – Aditya M P Feb 27 '13 at 05:54
  • 1
    Are we even talking objects here or just funcs all over the place? – Erik Reppen Feb 27 '13 at 05:54
  • possible duplicate of [How do you call the process of taking a part of a function and making an individual function of it?](http://programmers.stackexchange.com/questions/155330/how-do-you-call-the-process-of-taking-a-part-of-a-function-and-making-an-individ) – gnat Feb 27 '13 at 06:14
  • 3
    I don't think this is a duplicate of that question, I'm speaking a little more generally (and on a larger scale than a single function). – cmhobbs Feb 27 '13 at 06:58
  • 1
    erik - I am talking about objects and methods here. I've taken a couple of conditional messes and turned them into APIs. The problem comes when it's time to call the API. The first answer here may be exactly what I'm looking for, though. – cmhobbs Feb 27 '13 at 07:00
  • 2
    How on earth is that a duplicate? – MattDavey Feb 27 '13 at 11:10

5 Answers5

12

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.

Chris Travers
  • 1,134
  • 8
  • 11
  • I think you may have hit the nail on the head. The reason the glue exists may be due to the code that calls the interface I've created. I'm going to wait around for some more responses to see if we're missing anything, but I believe this one sums it up quite well. – cmhobbs Feb 27 '13 at 07:02
  • 1
    "execution paths meandering between modules and back, and between languages, with no rhyme or reason" - this reminds me of some modern OO practices as well. – user253751 Jun 29 '15 at 08:23
8

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

MattDavey
  • 7,096
  • 3
  • 31
  • 32
6

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.

pieter
  • 151
  • 5
  • 2
    Don't you get a glue-tree then? – Pieter B Feb 27 '13 at 11:39
  • 3
    @PieterB maybe, but it is easier to extract the different dependencies when you have the different tasks in different methods. You could do another refactoring pass after extracting the new methods. – pieter Feb 27 '13 at 11:53
1

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.

Karl Bielefeldt
  • 146,727
  • 38
  • 279
  • 479
0

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.

Dr. Hans-Peter Störr
  • 1,393
  • 2
  • 10
  • 10