I have a confession to make - I have a hard time providing code reviews. I tend to nit pick. A lot.
I notice that everyone seems to have a different way of writing code. And sometimes it can be really interesting to see new techniques. But often it can just add a mess to a codebase. Over time, when different developers come in, mark the codebase with their particular style, and leave that codebase for whatever reason, you start to feel like a geologist digging through the layers of the earths' crust.
I mostly review code for Javascript codebases. It’s a great language, and I love it. But it can allow for some fascinating syntax and still work as expected. And when you add JSX all hell breaks loose.
I have a tendency to want perfection. But code isn’t meant to be perfect. Code is a medium by which to describe what a computer should do. You can have the best code in the world, and no one will use it because maybe it doesn’t do anything useful.
Code seems to have an ever decreasing half-life. By which I mean that as soon as you write some code, it starts to decay. It’s usefulness as code becomes less and less, even though it’s functionality may keep living on. Because code is meant to be read by humans , not machines. Machines could care less what your variable names are, or whether you are using a typed language or not. They will do exactly what you tell them to. Nothing more, and nothing less.
If you have worked on any system that you may consider as “legacy” you may know exactly what I mean. The larger a codebase gets, the less likely that someone will go in and clean it up. When someone has to add or fix a feature, they will likely go in and surgically update as little code as possible.
I have a colleague who recently told me that he is less and less inclined to write new code lately. This is because he has started to realize that the more code you write, the more bugs you produce. He is starting to see code, effectively, as a liability instead of a an asset.
I remember when I first started to transition into web development. I started learning Angular on my own. I learned all of the right ways to write Angular, and internalized the best practices. I even started a pretty ambitious side project to get used to working with the technologies that I wanted to and get my foot into the door at the right company. When I got hired at a company using Angular, I was so excited!
I came in with no real experience in web development. I came in with high expectations of my Angular knowledge. That was until I looked at the codebase. I was so lost 🙈. Part of it was my inexperience, but the other part was how they architected their codebase. It was not using the best practices that I had come to appreciate. When I learned Angular I thought that nothing should go on $rootScope — you might as well place things on the global scope 😱. This was apparently not a shared sentiment. And that was just a small piece of the gigantic puzzle.
This is when I truly learned that code is not written in a vacuum. Everyone brings different experiences and motivations to the way they write and architect codebases. In fact, this is something I am still learning today.
Let's get back to my code review struggle. How have I learned to deal with my own issues and provide valuable feedback without nit-picking everything?
-
I realize that the code isn’t as important as the function or feature that we are trying to provide.
When I start to review code I first ask does it do what we expect it to do, and are there tests to verify this? Are there any possible side effects? If the code does what it’s supposed to do, and there are no foreseeable side effects, then I will let go of any stylistic details or logical order. Most of the time it doesn’t matter if we “bail out early” unless we would be doing a lot of computation work otherwise. I may add some comments about readability sometimes, but I will give it a checkmark and move on.
-
Try to preempt the problem.
I was once told that the person who cares most will write the rules. In my case, I created a coding style guide with some initial guidelines based on code reviews I had done previously. I then elicited some feedback and asked other developers to add their own stylistic preferences if they had any. The key is to have everyone agree on the guidelines to get buy-in. This is a living documentation that can evolve as we decide better practices. But the best purpose of it is to have something to point to to say what we decided as a team how to write code for a particular codebase. It’s also a great resource to onboard new developers to the team.
-
Sometimes I still just nit pick things.
I don’t see nit-picking things as a blocker any more. I see it as a way to bestow my opinion where it may be served well — think constructive criticism. Sometimes it can be a learning experience for other developers, but I’ll admit sometimes it may be a hinderance. I have learned not to do it to developers who have expressed that they only want actionable feedback. And If I really cared that much, I would come in behind the developer and clean it up. But I don’t have time to do that, and nobody should — it undermines that developer and breaks down trust.
So what about you? Do you have a problem with perfectionism? As a nit-picker or nit-pickee? Let me know how you deal with it? I’m also open to pointers on how I can do better.
Top comments (0)