Refactoring... well, it depends
lessons learned, a book read, and some important thoughts to jot down
In one of my other blog posts, I wrote about reading as a source of learning for a software developer. Now, finally, I've managed to read my first non-beginner book about coding! It was Refactoring - Improving the Design of Existing Code by Martin Fowler (featuring some excellent writing from Kent Beck). It prompted a lot of deep thoughts about how I can improve and has twinned nicely with my current feedback at work, that I need to focus on making my code as efficient and DRY as possible. Below I'll write some ramblings around topics which the book covers, how my experiences relate, and what techniques and approaches can be used to make sure that code is refactored properly.
When to refactor? If your code smells
Refactoring uses the phrase 'code smell' to cover a large part of the book, but I've had other devs refer to it as a spider-sense, or just a general feeling of grossness when reading some code. What does it actually mean?
Well, generally what we're talking about with code smells are a group of things you see in code which should be big red flags. These are the telltale signs that the code someone has written needs more work. Maybe it's inconsistent variable naming, maybe it's lots of comments, or maybe there are methods which are really long and need splitting up. Code smells aren't actual errors or bugs, but more indicators of problematic areas which could eventually lead to bugs, reduced maintainability or other issues.
Martin Fowler puts 'number 1 in the stink parade' as code duplication. This is a prime example of a code smell, and it's clear why. Repetition not only makes code unnecessarily long to read but also allows more space for tiny bugs and errors. In my experience, this is also the most common code smell you're likely to be pulled up on, so always remember to write DRY (don't repeat yourself)
Refactoring, at its heart, is simply the act of reducing these code smells each time you come to review or re-write code. Alternatively, Chat-GPT puts it as: The process of making code improvements without changing its external behaviour, to enhance its internal structure, readability, and maintainability. (No I'm not afraid of using a good AI breakdown everyone once in a while)
What to refactor? The 'it depends' brigade
Whenever you're discussing or reviewing code, it's always a good idea to look at something from the two viewpoints possible - these often argue with one another by saying well, contextually, 'it depends'. One viewpoint is that all code should be as minimal and efficient as possible. The other is to try and make code which is easy to read, debug and understand as possible. These two are the yin and yang of coding.
The reason we have both schools of thought is because of context... These contexts can be narrowed down somewhat (in my mind at least) to the following two issues:
1. Very rarely do we work alone when working on a project.
2. Everyone is at a different skill level.
If every dev in a team was a perfect developer, had expert-level knowledge of every inch of the product you're working on, and never wrote code which had bugs, then maybe you needn't focus much on readability or understanding. Your team would never have come back to the old code, as it was perfect, and even if they did for a new feature, they would be able to understand it because they were perfect too.
Unfortunately in the real world, you have things like bugs, a mixed-ability team, and people who aren't aware of the context in which you wrote something. These are all reasons to make your code simpler, but perhaps at the cost of minimalism.
You'll have devs who won't understand your niche technique for something which reduces code by 1 line. Or you may have a bug which would be easier to fix if you'd used a variable name rather than a single-letter iterator when mapping or looping in a method. Perhaps when expanding some functionality, you may be better off leaving a comment above a complex method or allowing for a more verbose variable name because the context in which it's being used, or where it's from isn't very clear.
What I'm trying to get at here is, you need to have these things in mind when refactoring. When I check my code or review someone else's now, I look for these sorts of things. I try to focus on firstly: can I understand this? then secondly: If I were writing this, could I minimise the code? rename anything? and are any of the tests too complex? (in the latter's case, probably split up the methods being tested). It's important when refactoring to take both sides into account. Remembering that we live in a world of dev TEAMS is a really important factor when refactoring code.
When to stop? It's all about confidence
In the end, you'll always have difficulty knowing when to stop. You will likely always be able to improve some code if you spend long enough looking at it. But I'll come back to Kent Beck's words to explain when it's best to stop this feuding between the two spirits on your shoulders. You'll know you're getting it when you can stop with confidence. Refactoring is more of a debate than a science, but most important of all, if you can stop refactoring, feeling confident your code is fit for purpose, clean, simple and readable (and passes tests) you're most likely at the right point to call it a day.
Refactoring tests
I'm a frontend developer and (despite a lot of disagreement from the more creative end of the frontend community) testing is a huge part of what we do, even in the frontend world of pretty websites, rainbows, and iced coffee.
Tests by default force you to do a lot of repetitive actions. Click here, type there, check this button exists, and so forth. What you can do in frontend testing is to block these actions and allow these to be refactored (see what I did there) into nicely re-usable functions.
For example, this neat (imo) refactor I had a few weeks back on some reused logic for one of our graph components. Here we wanted to replicate the dragging-in of a node into a graph pane (React-Flow if anyone's interested in the library). This was used in a bunch of tests, with slight differences, like the type of node being dragged in and the coordinates. Notice as well I've put in an unusual js docstring as a description for this function? This is because, crucially, this function sits in a seperate utils file, away from the test files. Having to switch between files to get the context of the arguments prevents easy coding, so I added a quick description block and here we have what I think is a nicely written Cypress utility function.
/**
* Drags in a node of type nodename
* @param nodeName: string, the name of the node type
* @param x?: number, the x co-ordinate this will be placed on the graph canvas
* @param y?: number, the y co-ordinate this will be placed on the graph canvas
*/
export const dragNodeIn = (nodeName: string, x?: number, y?: number) => {
const dataTransfer = new DataTransfer()
const draglocator = `someClassName`
const droplocator = 'someOtherClassName'
const xCoord = x ? x : 0
const yCoord = y ? y : 0
cy.get(draglocator).first().trigger('dragstart')
cy.get(droplocator).eq(0).trigger('drop', xCoord, yCoord)
cy.get(draglocator).first().trigger('dragend')
}
Another easy win in frontend testing (well, in Cypress and Jest/React testing library) is the use of the built-in beforeEach() / afterEach() methods. These can be used to run, as expected, before and after each test in a block to minimise code repetition. This is probably the single easiest and most efficient way to ensure your tests are minimal and clean. Would recommend 100% trying to implement these where you can.
Don't forget
The last special mention today is also on testing, and also a consistent point in Refactoring. We always say things like 'testing gives you confidence that your code works as expected'. When refactoring, always remember to use tests as your benchmark. If you're re-working a big chunk of code which works, but just isn't the best it could be. First things first, write comprehensive tests and make sure they pass. Then make your changes. If the tests fail, then use this indicator that you've probably refactored something wrong. It also helps as a small morsel of TDD to add to your CV as well.
Refactoring is a really important part of coding, and it probably makes up nearly 50% of my day-to-day work. If I got things perfect the first time around, then I'd need to be paid a lot more than I currently do! But in seriousness, I'd only get there from years of refactoring. Each time is a learning opportunity, and each time just remember, you're not just rewriting something differently. You're actively making the code better for itself, yourself and the rest of your team. Refactoring, the invisible team member!