Perfect code is an illusion
Much of our programmer culture is built on the ideal of perfect code: code which not only works, but is also clean and elegant.Daniel Irvine · Nov 11, 2016 · code-reviews career
We take pride in constructing clever solutions to difficult problems. This perfectionism, however, can be detrimental to the success of the team because perfectionism often leads to personal disagreements.
There’s no globally accepted vision of what perfect code is. Everyone has a slightly different aesthetic for code, meaning we each have our own idea of what perfect code looks like. If we are all driven by perfectionism—the desire for our code to look exactly how we want it to look—then we will eventually run into disagreements with our teammates, as we each fight against each other to have the codebase look exactly the way we want it to look.
As I’ve matured as a programmer, I’ve found that an effective technique in avoiding team conflict is to stop aiming for perfect code. What follows are some examples of how to apply that concept to your own work.
Don’t get hung up on dogma
The only thing to demand of a codebase is that it works. A simple way to verify that is if it’s fully test-covered and that those tests are passing. Beyond that, every measurement of quality is subjective.
When you read other people’s code, try not to think how you would prefer it to look. Try not to rewrite it in your head. Let it exist just the way it is.
Keep your coding standards short or non-existent
Tabs or spaces? Two spaces or four? Same line or new line for your opening brace? I do not know of a single programming language that is immune to this type of debate. The standard way to combat this is to use written coding standards. They bring consistency to our code.
Unfortunately it’s difficult to form complete coding standards. There will always be grey areas that cause potential for disagreement. Naming, patterns, object modelling techniques, and so forth.
And the rules we do write down can sometimes spark a fire.
A team I once belonged to had the following rule in its coding standards: “no function should be longer than 7 lines of code.” In hindsight, our team would have been better off without this rule. While I still agree with the sentiment, the rule provoked a lot of confusion and debate. This one rule caused a plethora of pull request comments followed later by modifications. People needed constant reminders of it. Some of the team never believed in it. All in all, we spent a lot of time and energy upholding this rule.
That time could have been better applied pairing and improving the code together. There is a cost to every rule you maintain, and it’s likely you’ll still have disagreements despite all those rules.
Although I still write short methods—usually much fewer than seven lines—I don’t bother to write this rule down.
Rather than writing out rules, let the codebase be its own standard.
Don’t let pull requests hold you up
Often, I’ll merge pull requests even if there are obvious improvements that could be made to the code. I do that for two reasons. The first is that waiting for corrections can block other team members. The second is more subtle. It’s important that a code base should always remain malleable: meaning, ready for and expecting to change. The culture of the “perfect pull request” discourages this. It promotes the notion that code in the master branch is “golden” and should no longer change. If we instead allow imperfect code into master, we encourage higher rates of change. The team learns to always ask the question “is the code I’m looking at clean enough?”
Somewhat counter-intuitively, allowing imperfect code into master can actually cause an increase in quality.
So what is a better approach to reviewing pull requests?
My strategy is this. I read through the entire set of changes first, keeping note of anything that might be important. Then I’ll prioritize my feedback and limit my comments to at most three suggestions. Everything else is left alone.
I’m least likely to comment on style issues like misplaced whitespace or poorly indented parameter lists. If the code is malleable, someone is likely to clean the code at some future time. And in the meantime, these stylistic issues aren’t going to hurt anybody.
Let go of the grand vision
For anything more than a few dozen lines of code, perfection is in the eye of the beholder. If you expect everyone to solve problems in exactly the same way you do, then you are mistaken. If you hold a grand vision of what your codebase should look like, then you will be disappointed.
Give your teammates the space to design and code as they feel appropriate. Encourage everyone to take an equal role in designing the system.
And when your teammates write code that looks different from what you’d like it to be, don’t argue with them. Remember that maintaining healthy working relationships within your team is valuable in the long-term. So perhaps it’s okay to sacrifice your personal vision of quality.
I encourage you to take some time each day for some personal retrospection of your development technique. Think about how effective you are each day, for yourself and for your team. What works this month may no longer work next month. This is especially true as teams grow in skill from novice to expert, so make sure to keep hunting out the parts of your process that are beginning to hurt more than they help.