Refactoring: How do I even start?
Let's face it: working with sloppy, legacy code is inevitable. Whether from a rushed developer that's long left the company, or even just you from 6 months ago, eventually you are going to find yourself with big mess of muck that you suddenly need to modify. You have no idea what's going on and there's a bug that cannnot be consistently reproduced. Also, you have until the end of the day and as far as your manager is concerned, refactoring is that thing you learned in middle school to break down numbers.
 |
| Also, this guy is your boss. |
You are probably a little overwhelmed and feeling pressure from people who don't fully understand the extent of the issue. Maybe you want to build a time machine and stop the original author from doing this (which right now may seem like the easier solution).
It's ok, take a moment to close your eyes and take a deep breath. This too shall pass.
 |
| Or scream. That's a good one too. |
If you had more time you might be able refactor a lot of the code and maybe implement some design patterns. And if that was the case, check out sourcemaking.com for an extensive list of ideas. But you don't always have that time; sometimes you just need a little help making the code make SOME sense.
Easy ways to clean up code
The key is to start small and stay within a small scope. It'll be easier to keep track of things and staying within a small scope will lessen the chance of side effects. Here are couple of easy, non-invasive techniques to help you make sense of your code and clean it up at the same time
Using and Renaming Variables
Let's say you find a nested for-loop like this:
for( Integer i = 0; i < list1.length(); i++ )
{
for( Integer x = 0; i < list1[i].list2.length(); x++)
{
for( Integer y = 0; i < list1[i].list2[y].list3.length(); y++)
{
if( list1[i].list2[x].list3[y].Name == list1[i].Name && list1[i].list2[x].list3[y].Name == list1[i].list2[x].Name )
{
list1[i].list2[x].list3[y].Name += ' III';
}
}
}
}
You have no idea what these lists are and the chaining is making it difficult to keep track of what is being compared and what is being assigned. But, what we do know is that these lists are related in some kind of hierarchy. So let's rename some of these variables and place repetitive references into new variables
for( Integer i = 0; i < list1.length(); i++ )
{
Object grandparent = grandparent;
List<Object> parents = grandparent.list2;
for( Integer x = 0; i < parents.length(); x++)
{
Object aParent = parents[x];
List<Object> children = aParent.list3;
for( Integer y = 0; i < children.length(); y++)
{
Object aChild = children[y];
if( aChild.Name == grandparent.Name && aChild.Name == aParent.Name )
{
aChild.Name += ' III';
}
}
}
}
A step forward. In our mental map of the code we no longer need to tranverse any hierarchies to figure out what is being operated on and there's a little more context to our variables.
Composing Methods
One of the keys to understanding messy code is reducing the amount of information you need to keep track of in your mental map. Here's the method we worked above with the rest of the method:
public void processList( List<Object> masterList ){
List<Object> list1 = new List<Object>();
for( Integer z = 0; z < masterList.length(); z++ )
{
if( masterList[z].list2 != NULL masterList[z].list2.length() > 0 )
{
for( Integer y = 0; y < masterList[z].list2.length(); y++ )
{
if( masterList[z].list2 != NULL masterList[z].list2.length() > 0 )
{
list1.add(masterList[z]);
}
}
}
}
for( Integer i = 0; i < list1.length(); i++ )
{
Object grandparent = grandparent;
List<Object> parents = grandparent.list2;
for( Integer x = 0; i < parents.length(); x++)
{
Object aParent = parents[x];
List<Object> children = aParent.list3;
for( Integer y = 0; i < children.length(); y++)
{
Object aChild = children[y];
if( aChild.Name == grandparent.Name && aChild.Name == aParent.Name )
{
aChild.Name += ' III';
}
}
}
}
}
After looking at the code for a little bit, you realize that the first for loop is building a list of objects that have both children and grandchildren in its hierarchy. Also you remember that the second for loop is adding a suffix to the grandchildren's name. Instead of having to remember what lines do what, let's move some of this logic out into different methods:
public void processList( List<Object> masterList ){
List<Object> list1 = filterObjectsWithGranchildren( masterList );
appendSuffixToGrandchildren( list1 );
}
private List<Object> filterObjectsWithGranchildren( List<Object> listToFilter ){
List<Object> filteredList = new List<Object>();
for( Integer z = 0; z < listToFilter.length(); z++ )
{
if( listToFilter[z].list2 != NULL listToFilter[z].list2.length() > 0 )
{
for( Integer y = 0; y < listToFilter[z].list2.length(); y++ )
{
if( listToFilter[z].list2 != NULL listToFilter[z].list2.length() > 0 )
{
filteredList.add(listToFilter[z]);
}
}
}
}
return filteredList;
}
private void appendSuffixToGrandchildren( List<Object> grandparents ){
for( Integer i = 0; i < grandparents.length(); i++ )
{
Object grandparent = grandparent;
List<Object> parents = grandparent.list2;
for( Integer x = 0; i < parents.length(); x++)
{
Object aParent = parents[x];
List<Object> children = aParent.list3;
for( Integer y = 0; i < children.length(); y++)
{
Object aChild = children[y];
if( aChild.Name == grandparent.Name && aChild.Name == aParent.Name )
{
aChild.Name += ' III';
}
}
}
}
}
Now our original method calls two private methods. The logic is still the same, but now we can quickly understand what our original method does without having to read through all of the logic.
When it comes to refactoring, this is only the beginning. However, these are my go to strategies when I need to make changes to seemingly nonsensical code and have no idea where to start. Work piece by piece and you'll soon find yourself with a whole.