Firstly, let’s explain what refactoring code means. It’s like a music remix. You know how you hear a really good song, and five to ten years later some random artist, or sometimes the same artist, makes a remix of it? And the remix is even better than the original. It’s almost always better because it’s been updated. It’s got the new beats and sounds, but with the best bits of the original track.
That’s what refactored code is. It’s better because it’s updated, with the best bits of the previous code. It’s got the new design patterns, frameworks, libraries – but, most of all, it’s got the new you as a more experienced developer. Just like any artist who continues their craft, you improve.
Refactoring code before it becomes a burden is one of the trickiest and best examples of software development best practices. There is a really important differentiation in there. It has to happen before the task of refactoring becomes burdensome.
If you’ve been in the development field for many years, you’ll have experienced refactoring an ancient code base or an old, unmaintained application. Once you hit a certain level of refactoring, the work becomes tedious because it’s such a huge amount of work. It’s no longer fun. It turns into busy work as you update seriously outdated code, requiring a lot of effort and man hours. Once it turns into busy work, you will find a way to script it or automate it.
And so, instead of spending nine hours manually editing files, you build a script in two hours to make all of the changes in three seconds. The scripting bit is actually quite fun. The problem is, it’s the only fun part in a situation like that.
You shouldn’t have to spend two hours building that throw-away script, because the code should have been refactored well before that point. Refactoring code shouldn’t be tedious.
A good rule of thumb is, if you notice that your code is really, really obviously in need of a refactor, then you’ve probably waited too long. By the time you start, it’s already well past the point where it became tedious. So, I guess what I’m trying to say is, refactor and refactor often, because the cost of refactoring too much is a lot less than refactoring too little.
This is a timely reminder for me, because I witnessed some coworkers make this early refactoring decision recently. It was awesome. There are three of them on a new project right now. (I’m not on it, and don’t have anything to do with it at this point, but it would be a fun project to work on.) It uses some frameworks outside of our core stack, so they get to brush up on some interesting libraries at the same time.
The other day, they all suddenly stopped what they were doing. It was about 5pm and you could tell they were a bit tired from the day’s work. This is early on in the lifetime of the project and a lot of developers, especially the inexperienced ones, would never think to refactor code so soon. They would think that making those updates means admitting they failed at some point, when that is not the case. The team stopped and said “this code is too complicated, it’s too deep.”
The cyclomatic complexity of the code is what they were talking about. They felt like it shouldn’t be this hard to follow so early on. And so one of them said, “I think we need to refactor this code.” It’s such a hard thing to admit, but it’s the best admission you can make on a project. It’s an admission that saves a whole lot of time and effort down the track. Seeing this unfold first hand was exciting because I know the feeling (once you’ve made the decision). So, not only do they get to work with some cool new tools, but they have a nice healthy refactor to do. Should be fun times ahead.
It’ll be fun because you’re taking something that has already been built and improving it. And if you have good test coverage, you can make changes to your heart’s content.
Refactoring – Who to Ask, and Examples
Wondering how to improve a particularly intricate chunk of code? There are patterns for that. Or, if you work in a larger team you probably have coworkers that would be happy to help. Don’t be afraid to ask! I’m not a proponent for always pair programming, but there are situations where I find it beneficial to pair programme. This would be one of those situations. If you work remotely, there are even solutions to pair over the intertubes! If you use Slack, then you have built-in support for screen sharing – they took my favourite code collaboration software, Screen Hero, and integrated it with Slack. I’ve also used Zoom to screen share but it doesn’t have all the bells and whistles. Or you could try the plethora of other screen share applications.
I’ll end with a couple of concrete examples of when to refactor. The first example is an easy one to spot. This is a bit of Loopback model code. I needed to reuse a chunk of code in related polymorphic classes. So I extracted the function to DRY it up. Variable names have been changed to protect the innocent.
Before
// bumble-bee.js
...
BumbleBee.beforeRemote('prototype.patchAttributes', async (ctx) => {
if (Object.keys(get(ctx, 'args.data.data.relationships', [])).some(k => k.endsWith('honey'))) {
const honeyId = ctx.instance.id
const [lavender, manuka, sunflower] = await Promise.all([
LavenderHoney.find({where: {honeyId}, fields: {id: true}}),
ManukaHoney.find({where: {honeyId}, fields: {id: true}}),
SunflowerHoney.find({where: {honeyId}, fields: {id: true}})
])
ctx.args.data.data.relationships['lavender-honey'] = {data: lavender.map(t => ({id: t.id.toString(), type: 'lavender-honey'}))}
ctx.args.data.data.relationships['manuka-honey'] = {data: manuka.map(t => ({id: t.id.toString(), type: 'manuka-honey'}))}
ctx.args.data.data.relationships['sunflower-honey'] = {data: sunflower.map(t => ({id: t.id.toString(), type: 'sunflower-honey'}))}
}
})After
// bumble-bee.js
BumbleBee.beforeRemote('prototype.patchAttributes', async (ctx) => {
await BumbleBee.app.U.jsonapiRelationshipFix(ctx)
})
// honey-bee.js
HoneyBee.beforeRemote('prototype.patchAttributes', async (ctx) => {
await HoneyBee.app.U.jsonapiRelationshipFix(ctx)
})
...
//bee-utils.js
...
async jsonapiRelationshipFix (ctx {
const fixRelationship = (modelName, relations) => {
ctx.args.data.data.relationships[modelName] = {data: relations.map(t => ({id: t.id.toString(), type: modelName}))}
}
const fixNeeded = Object.keys(get(ctx, 'args.data.data.relationships', [])).some(k => k.endsWith('honey'))
if (fixNeeded) {
const honeyId = ctx.instance.id
const [lavender, manuka, sunflower] = await Promise.all([
LavenderHoney.find({where: {honeyId}, fields: {id: true}}),
ManukaHoney.find({where: {honeyId}, fields: {id: true}}),
SunflowerHoney.find({where: {honeyId}, fields: {id: true}})
])
fixRelationship('lavender-honey', lavender)
fixRelationship('manuka-honey', manuka)
fixRelationship('sunflower-honey', sunflower)
}
}
...The second example is not as obvious at first glance. This is a chunk of Ember model utility code. The switch statement was getting a bit too big for its britches. To clean this up I used the Strategy pattern. Again, variable names have been changed to protect the innocent.
Before
// bee-utils.js
...
beeAction (bee, action, ...actionArgs) {
let payload, body, filesForUpload
const modelName = bee.constructor.modelName
const url = `/${modelName}s/${bee.id}/${action}/`
const sendNoticeData = (content) => {
return this.post(url, { data: JSON.stringify(content) })
.then(results => get(this, 'store').pushPayload(results))
.then(() => RSVP.all([
bee.belongsTo('beeMeta').reload(),
bee.hasMany('wiggles').reload()
]))
.then(() => get(this, 'store').peekRecord(modelName, bee.id))
}
switch (action) {
case 'gathering-errors':
return get(this, 'getTask').perform(url).then(body => body.errors)
case 'wiggle':
const [queenId] = actionArgs
return this.post(url + ownerId).then(results => get(this, 'store').pushPayload(results))
case 'waggle':
return this.post(url)
case 'modify-dance':
const [pollinationDate] = actionArgs
body = { data: { attributes: { pollinationCompleteDate: pollinationDate }, type: 'custom' } }
return sendNoticeData(body)
case 'complete-gathering':
case 'complete-sap-search':
case 'stop-sap-search':
case 'dance-party':
[payload, filesForUpload] = actionArgs
return this.compositeUpload(url, payload, filesForUpload)
.then(() => get(this, 'store').peekRecord(modelName, bee.id))
case 'start-sap-search':
[payload] = actionArgs
body = { data: { attributes: payload, type: 'custom' } }
return sendNoticeData(body)
default:
return get(this, 'postTask').perform(url, { data: JSON.stringify(bee.serialize()) })
.then(results => {
get(this, 'store').pushPayload(results)
})
}
}
...After
// utils/bee-utils.js
...
beeAction (bee, action, ...actionArgs) {
const modelName = bee.constructor.modelName
const url = `/${modelName}s/${bee.id}/${action}/`
const actionMap = {
'gathering-errors': bee.gatheringErrors,
'wiggle': bee.wiggle,
'waggle': bee.waggle,
'modify-dance': bee.modifyDance,
'complete-gathering': bee.compositeUpload,
'complete-sap-search': bee.compositeUpload,
'stop-sap-search': bee.compositeUpload,
'dance-party': bee.compositeUpload,
'start-sap-search': bee.startSapSearch
}
if (actionMap[action]) return actionMap[action](url, actionArgs)
return bee.defaultAction(url)
}
…
//models/bee.js
...
export default BeeBase.extend({
...
_sendNoticeData (url, content) {
return this.post(url, { data: JSON.stringify(content) })
.then(results => get(this, 'store').pushPayload(results))
.then(() => RSVP.all([
this.belongsTo('beeMeta').reload(),
this.hasMany('wiggles').reload()
]))
.then(() => get(this, 'store').peekRecord(modelName, this.id))
}
gatheringErrors (url) {
return get(this, 'getTask').perform(url).then(body => body.errors)
},
wiggle (url, actionArgs) {
const [queenId] = actionArgs
return this.post(url + queenId).then(results => get(this, 'store').pushPayload(results))
},
waggle (url) {
return this.post(url)
},
modifyDance (url, actionArgs) {
const [pollinationDate] = actionArgs
const body = { data: { attributes: { pollinationCompleteDate: pollinationDate }, type: 'custom' } }
return _sendNoticeData(url, body)
},
compositeUpload (url, actionArgs) {
const [payload, filesForUpload] = actionArgs
return this.compositeUpload(url, payload, filesForUpload)
.then(() => get(this, 'store').peekRecord(modelName, this.id))
},
startSapSearch (url, actionArgs) {
const [payload] = actionArgs
const body = { data: { attributes: payload, type: 'custom' } }
return _sendNoticeData(url, body)
},
defaultAction (url) {
return get(this, 'postTask').perform(url, { data: JSON.stringify(this.serialize()) })
.then(results => {
get(this, 'store').pushPayload(results)
})
}
...
})If you’re looking to hone your refactoring skillz, there is plenty of material online. But by far the best thing you can do is practice. Good luck and happy coding!
Further Reading on When to Refactor
Other blog posts:
- https://codeclimate.com/blog/when-is-it-time-to-refactor/
- https://spin.atomicobject.com/2017/11/16/when-to-refactor-code/
A book by the well informed Martin Fowler:
