Refactoring old code

Featured on Hashnode
Featured on daily.dev
Listen to this article

As a developer, you write code (duh), but this code might be outdated in one, three, five years.

I think it's our responsibility to maintain this code if the budget allows it.

In this case, the code has been written by my colleague pre ES6 times. I'm now asked to write an extension on this codebase.

When looking at the old code, I noted there was some legacy code that was using loops and wasn't really efficient with the tools we have nowadays.

Introducing the old code

In the example we are looking at, we have the following datasets.

const data = {
  Angular: 3,
  React: 1,
  Vue: 2,
  Next: 1,
  HTML: 2,
  Other: 3
};
const colors = ['#d17a29', '#da9554', '#e3af7f', '#edcaa9', '#f6e4d4', '#204e77'];

The goal is to get an output like this:

[
  ['Angular', 3, '#d17a29'],
  ['Other', 3, '#204e77'],
  ['Vue', 2, '#e3af7f'],
  ['HTML', 2, '#f6e4d4'],
  ['React', 1, '#da9554'],
  ['Next', 1, '#edcaa9']
];

Looking at the old code, which is the following:

let sortable = [];
let index = 0;

for (let temp in data) {
  sortable.push([temp, data[temp], colors[index] ? colors[index] : '#D3D3D3']);
  index++;
}

sortable.sort(function(a, b) {
  return b[1] - a[1];
});

The person achieved the exact goal, using a loop and manual plus on an index. Perfect solution, but perhaps we can make it even better?

Refactoring the code

Immediately is was thinking of using the Map method to map the data into the desired format.

But we can't use the Map method on an object?

Right, so let's convert this object into an array.

const newOutput = Object.entries(data);

This will give us the following array.

[['Angular', 3], ['React', 1], ['Vue', 2], ['Next', 1], ['HTML', 2], ['Other', 3]];

Wow, that already halfway there, it's not sorted yet, and we are missing the color, but it's a really good start.

Note: With this, we omitted the manual push into a new array.

Now let's try and add the color based on an index. Start by adding a Map method to the entries.

const newOutput = Object.entries(data).map(item => item);

This will return the exact same as what we had. But did you know we can add an index to this?

const newOutput = Object.entries(data).map((item, index) => item);

Another cool thing we can do inside this map is to break down the item into separate variables.

const newOutput = Object.entries(data).map(([title, amount], index) => title);

The above example will return:

['Angular', 'React', 'Vue', 'Next', 'HTML', 'Other'];

You might see where this is going now, so instead of returning just the title, we can return an array.

const newOutput = Object.entries(data).map(([title, amount], index) => [
  title,
  amount,
  colors[index] || '#fff'
]);

There we go. We added the color to our output.

[
  ['Angular', 3, '#d17a29'],
  ['React', 1, '#da9554'],
  ['Vue', 2, '#e3af7f'],
  ['Next', 1, '#edcaa9'],
  ['HTML', 2, '#f6e4d4'],
  ['Other', 3, '#204e77']
];

The last thing we need is to have the array sorted based on the number (second variable).

const newOutput = Object.entries(data)
  .map(([title, amount], index) => [title, amount, colors[index] || '#fff'])
  .sort((a, b) => b[1] - a[1]);

Let's check the output now:

[
  ['Angular', 3, '#d17a29'],
  ['Other', 3, '#204e77'],
  ['Vue', 2, '#e3af7f'],
  ['HTML', 2, '#f6e4d4'],
  ['React', 1, '#da9554'],
  ['Next', 1, '#edcaa9']
];

We did it. We refactored the old code to a single line using combined methods.

I hope you see how refactoring code makes sense and how the thinking process works.

Thank you for reading, and let's connect!

Thank you for reading my blog. Feel free to subscribe to my email newsletter and connect on Facebook or Twitter

andrei's photo

Man, good job on writing and puttin you out there, but I made an account just to reply on this. Hate to see things overused just because they're shiny.

I noted there was some legacy code that was using loops and wasn't really efficient with the tools we have nowadays.

You replaced one completely fine loop with two loops and a ton of allocations. for is not freaking outdated. I think you meant to say "pretty" or "easier to follow", because the "modern" version is way, way less efficient than the original.

Chris Bongers's photo

Hey Kernel

I might have actually better called the article "Modernizing your code" since the refactor wasn't a hard need, the code is working as is, and nothing wrong with it.

This would just be the modern-day approach.

Murilo Boareto Delefrate's photo

The article is well written and it has a very good knowledge about how to refactor a code, but I would do this refactoring in a completely different way. If you have a static list of items (that is short enough as in the example), why don't you simple create a variable that will be the final resolve as you expect? Why using loops and so on while you could have a simple

const newOutput = [
  ['Angular', 3, '#d17a29'],
  ['React', 1, '#da9554'],
  ['Vue', 2, '#e3af7f'],
  ['Next', 1, '#edcaa9'],
  ['HTML', 2, '#f6e4d4'],
  ['Other', 3, '#204e77']
];
Luiz Filipe da Silva's photo

Murilo Boareto Delefrate this article just gives us an example. Obviously, in a real world case, the algorithm would be complex and the variables could be used in other places along with the code.

romeo adjam's photo

thank you for this great post

Chris Bongers's photo

Thank you Romeo! Glad you like it

Chris Tanner's photo

" Refactoring is intended to improve the design, structure, and/or implementation of the software, while preserving its functionality."

I don't think you've improved the design, structure, or implementation of this code. You've taken some clear, easy to read code, and made it harder to read. You've decreased the number of lines being used, but what's the benefit of that? We aren't charged by the number of lines we use.

Please, please don't do this.

Chris Bongers's photo

I tend to disagree with you there Chris.

This book is specified on PHP, but the concept has some valid points.

adamwathan.me/refactoring-to-collections

However: I might have better called this article: "Modernizing your code" instead of refactor

Chris Tanner's photo

Chris Bongers

Let's look at the pros and cons of this modernising.

Pros:

  • Uses modern technology, which is good, because... modern?
  • Uses less lines of code.

Cons: Readability - in an ideal world everyone would be up to date with ES6, but in reality most people are wayyy behind. I bet if you took a survey of software developers and asked them if they knew what a for each does vs Object.Entries, you'd have a lot more that understand the former. If the goal is to have clear code with a low probability that someone else modifying the code will make a mistake, going will the former will be more likely to achieve that. It's much safer to go with the lowest common denominator.

Bugs - Every time you modify code, there is the potential to add new bugs to the software. You have to weigh up this risk vs the benefit of making the changes. Usually code changes are either making the code more efficient, easier to read, or adding some new feature, which trumps the risk of creating bugs.

Efficiency - map is less efficient than for each, so this code will most likely be slower

Time - This took some amount of time and thought to implement, and presumably test. Since the code produces the same outcome, the time could have been better spent on producing value.

Debugging - It's harder to debug your script since it's so condensed. The old one you can just set a breakpoint on the line you want to debug, the new one you have to set a breakpoint on the line you want to debug, then the subsection. (Support for this has only just been added to vscode, support in other IDEs might not be there)

Kevin Pliester's photo

I wish I could do it all at our place too. Unfortunately, most (german) visitors to our site still use Internet Explorer, because of SAP or other requirements from their companies.

I always have to make everything as old as possible so that it works everywhere. These are often customers from the legal field, funnily enough - who then still use Internet Explorer.

Thank you for your post, I find it quite useful, especially to refresh my brain cells a bit.

Chris Bongers's photo

Hey Kevin that struggle is so real, my previous company had to support up to IE9

Akash Raju M's photo

You don't have to write old code, you can use a JavaScript Compiler like babeljs.io