Make Bad Code Look Bad
If you’re working with legacy code, chances are some of the areas of code you’re in charge of have a design that is less than ideal.
But if you’ve been working with it for a while, you may be able to navigate this code with ease, and maybe you don’t even see its little weirdnesses any more.
This is a dangerous situation. It makes it easy to overlook some design issues and to fail to recognise that it is difficult to integrate new developers into this code.
There is one thing we can start with: transfer our knowledge into naming.
Making an impact with your knowledge
Consider the following big class:
class rvDTP_CLASS
{
public:
void soGR(int get);
virtual int getObjectID();
void evaluate(EvaluationParameters& parameters);
// ...
// lots of other member functions...
// ...
};
This is inspired from a true story (although the code has been anonymized).
Imagine you’re inheriting that code. At the beginning, all this means nothing to you. When you join the project you wonder, puzzled, what does soGR means?
After making a few features and fixes in the surrounding code, you come to realise that soGR is the function that interacts with the database. It allows to save or load the contents of the object, and the int it takes indicate whether it should save or load. A bit of the class starts making sense!
And then you live on your life, exploring and working on other unrelated parts of the code.
Later on, you come back to the rvDTP_CLASS, from a code handling UI. This code is asking the class the result of getObjectID(). A bit of code exploration makes you understand that the various derived class from rvDTP_CLASS return their own ID, and this ID correspond to a type of display screen.
getObjectID lets the UI layer know which screen to load in order to display the object corresponding to a derived class.
You can now map every class to something in the UI. Great, that gives you a concrete image of what rvDTP_CLASS and its derived classes represents. In fact they represent the product (let’s assume that “product” means something in the domain of this application).
Months pass, and little by little you get a finer and finer understanding of the class, its responsibilities and each of its member functions. You possess knowledge.
New people join the team, and they’re completely bamboozled when they see the rvDTP_CLASS and its member functions for the first time. To help them make the fix they need, you tell them what you learned back then: soGR interacts with the database!
But this is not a good way to pass knowledge, as I’ve come to realise, and I’ve certainly been guilty of giving this “help” to more junior developers.
Passing on the knowledge is good. But passing on to the next person is suboptimal. What you want is passing it on to the code.
Naming things for what they are
If you’ve been working with a given area of code for a long time, you may no longer realise what it looks like to someone seeing it for the first time. But their point of view is crucial to assess how expressive the code is.
So when someones asks: “what is soGR“, the best way isn’t to answer that it handles the interaction with the database to save and load the object. A better way is to bake this in the code:
class rvDTP_CLASS
{
public:
void loadOrSave(int load);
virtual int getObjectID();
void evaluate(EvaluationParameters& parameters);
// ...
// lots of other member functions...
// ...
};
But this new name can make you cringe. A function called loadOrSave? This looks like a terrible name. It’s screams that it’s in charge of two responsibilities!
While you’re at it, rename everything:
class Product
{
public:
void loadOrSave(int load);
virtual int getScreenID();
void evaluate(EvaluationParameters& parameters);
// ...
// lots of other member functions...
// ...
};
And this makes you cringe even more. A class that mixes DB, UI and business logic? This is the kind of counter-example we see in design books!
The thing is that it’s been there all along, but it was disguised behind mysterious acronyms.
Now you no longer have to explain what rvDTP_CLASS, soGR or ObjectID means. But you have a new type of problem.
You unearthed skeletons
Until now, the code looked mysterious. But making the renaming fix made it visible that the code has a poor design. If you’re in charge of this code (and you should consider yourself in charge of any code you work with), it can make you cringe even more.
But it is important to keep in mind that unveiling problems is a step towards fixing them.
Indeed, now you can better see the big picture of the class. Until now, your knowledge may have been composed of bits and pieces gleaned from individual explorations of the code, but after renaming its members, you see the class for what it is, as a whole.
This is the right time for deciding what to fix and when to fix it. For example in this class we’ve seen two possible improvements:
- separate the load from the save function
- separate the UI, DB and business logic of the class
When you do that, everything should become easier: the code will become easier to understand, explain, and unit test.
But instead of jumping into refactoring, make sure you’ve renamed as many things as you can in the class. Once you understand the code, renaming is fast. The more you can rename, the more quickly you can get an overview of design issues of the code.
Once you have a list of improvements to make, this constitutes a technical roadmap for this class.
Then comes the step of prioritisation: is this class the most important in your code? Are there other classes for which you should also create a roadmap?
If you’re working with legacy code, there may be more skeletons than you have the resources to deal with, so you need to choose which ones will bring the most value. To evaluate the value of refactoring projects and choose the best ones, refer to chapter 12 of the Legacy Code Programmer’s Toolbox.
The power of naming
Once you have acquired knowledge, transfer it into correct names in your code. The operation of renaming is typically fast and with very little risk.
Even if naming doesn’t improve the behaviour of the code per se, it is the first step towards understanding, integrating new joiners, decoupling, modularisation, unit testing, and ultimately an application that evolves faster and has less bugs.
You will also like
- A Concrete Example of Naming Consistency
- How to choose good names in your code
- The Legacy Code Programmer’s Toolbox
- Making Wrong Code Look Wrong (Joel on Software)
Share this post!
