I’ll forgo an intro, here’s the idea: every Friday morning, as soon as your fingers meet your keyboard, check out a new branch and go looking for something to fix. Fix it, merge it, go back to whatever you were doing.
Why tidy code is important
You may have heard of the Broken Windows Theory. It goes something like this: “If a window is broken and left unrepaired, people will conclude that no one cares and no one is in charge.” It was behind Mayor Giuliani’s success in cleaning up New York city and making it the pristine wonderland that it is today.
It’s a cute story: the year was 1993, I was having brunch with Rudy (Mr Giuliani to you) and telling him that each Friday he should offer all the homeless people a haircut and a shave and maybe wash some graffiti off trains. He just pointed at me with a couple of index fingers and said “you” in his thick New York accent; he was pleased.
Many years and many fewer murders later, I realised that my crime-fighting super-powers could be applied to code as well.
The actually-serious point I’m making is that when a code base is in great shape, it’s quite easy to keep that way, and it’s easy for new members of the team to know how to write tidy code as well. No matter what your definition of ‘tidy code’ is.
What is ‘tidy’ code?
To tidy up your code, there’s the obvious low hanging fruit of your tech-debt register or those TODOs sprinkled throughout your code.
If you have neither of these (ಠ_ಠ), here’s a few things that may be worth a tidy up:
- strings used in multiple places that could be constants/enums
- multiple functions doing similar things that could be a single function
- 2,000 line files that could be multiple smaller modules
- linting errors (or no linting)
- missing unit tests
- repeated values (e.g. padding, colors, box-shadows in CSS) that could be variables (e.g. using Sass/Less)
- file names that no longer represent what’s happening in the file
- images that are no longer referenced by any code
(AKA a list of things that I need to fix up in my own projects.)
Why not just fix code as I go?
For a lot of issues, fixing them as you go is a good idea. But there’s a lot to be said for keeping branches focused on what the branch needs to do. For example, when it comes time to do a code review, it can be harder for the reviewer to assess what you’ve done if it’s mixed in with a bunch of changes unrelated to the task at hand.
This is especially true if you rename/move a file you have made changes to. The commit won’t show the changed lines, it will show a removed file and new file separately.
Time for some structured guidance…
It’s important to agree on how much time to spend on Tidy Fridy. Half a day is a nice round 10%, but perhaps an hour or two makes sense for you. If you’ve just inherited an ageing codebase that needs some love, maybe a whole day is required for a while.
When it comes to the nitty gritty, Tidy Fridy might look quite different for different teams. So here I’ll layout a few recipes for how things might work at different scales.
1. The A team
Perhaps you’re a cog in (or valued member of) a well-oiled machine. You have say, 10 developers, you do agile, you follow feature branch or gitflow, you do code reviews, you do test automation, QA, CI, CD.
With such a team, it is important that everyone is on board with Tidy Fridy. Aim to have all work done, reviewed, QA’d and merged back into your master branch within the allotted time.
So, Friday morning rolls around, you work out what you want to tidy up. You do the work, push the branch and request a code review.
When you’ve done this, immediately pick up a code-review for a Tidy Fridy task from fellow developer, or help out with QA.
Because of the nature of these tasks — often DRYing up code from disparate areas of the code base — the broader the knowledge of the code base the better. So even though these tasks might be small, code reviews are no less important. Frexample, imagine Chris picks up a task that combines two functions, formatAsCurrency(str) and toStringWithCommas(str). Chris combines them into a formatString(str, options) function. Now you pick up the code review and you recall that there’s another function elsewhere that does something similar, and it uses the native toLocaleString() with the new options parameter. Chris didn’t know about this function, so you point it out. (Optionally followed by several minutes of severe admonishment for failure to achieve perfect knowledge of the entire code base.)
Next up, because this is a team effort, your QA stars are on standby. Many Tidy Fridy tasks will be zero-impact (e.g. renaming a file should have zero impact on the functioning of the application), however if you’re combining 20 different shades of blue in your UI down to 3 there should be differences. If you have automated visual regression testing (niiiice), you may want to group all your zero-impact branches together and push other branches through one-by-one.
When QA has passed the changes, get them merged into your master branch, STAT. When you’re finished with Tidy Fridy you will return to whatever branch you were working on the day before; if all goes according to plan you will be able to rebase that branch onto master straight away (or merge master into your branch if that’s how you roll).
It’s important these steps are all done as a single, contiguous chunk of work. It will take the pain out of having to rebase/merge 10 different tidy-up branches one at a time.
2. The startup
Maybe your team isn’t quite so structured. Perhaps you don’t do code reviews or QA. Perhaps you have a foosball table and do all your work in the master branch. That’s totes OK, it doesn’t mean you can’t benefit from Tidy Fridy.
I can’t say enough good things about code reviews, but if that’s not part of your process you can get a similar effect from a quick chat over slack or hangouts or even mouth-to-ear protocol.
DEV 1: Hey I’m gonna move our 237 lines of distributed button CSS into a single Sass mixin, y’all cool with that?
DEV 2: Oh that’s going to feel so nice. I’ve noticed we have a .btn--disabled class but we could just style that with a button[disabled] selector and remove the additional logic for adding the class.
DEV 3: Oh Dev 1 that’s super. I’ve been thinking we should require [role=button] in order to get button styles applied to something other than a button element, like those clever folks at eBay do. What do youse guys think? [Note: mouth-to-ear protocol doesn’t support hyperlinks.]
DEV 1: Great suggestions, you two are really great. I mean really great. Like, maybe … maybe we could be outside-of-work friends?
The key part is that in trying to create a more coherent code base, the broader the knowledge of the code the better you’re able to pull it all together.
3. The lone cowboy
Maybe you don’t work on a team at all. Maybe you’re the lone web developer at an ad agency and put together sites with fairly short lifetimes. Maybe you’re working on a solo project. Maybe people just aren’t very fond of you, or vice versa. Here it’s harder to see the benefit of tidy code, but it’s every bit as important to put aside a regular time to stay on top of things.
Writing code one small task at a time without outside input is like tiling a bathroom with thousands of little green, blue and aqua tiles. You get down and grouty, laying out the tiles with random abandon, and that’s great. But every now and then you should step back and make sure you haven’t accidentally created a portrait of Stalin on your bathroom floor.
I could tell you that you should balance perfectly tidy code with getting stuff done, but you already know that. And you already know that I already know that.
I hope the recipes above are of some use, but don’t fret too much about the details up front. Just try it out one week then discuss what worked, discuss what didn’t, and adjust to taste.
If you like the idea of Tidy Fridy but don’t want to do it on Friday, you might prefer TODO Toosday.