When I Review Code
How I carry out code reviews
October 29, 2016
I am behind on my writing. Got busy in the last few weeks being in a shuffle of projects. Different challenges, different environments, different tech stacks, my kind of fun. And if there is one thing I like about it this time, we now officially incorporate code reviews in our workflow.
Yay! Code reviews! Except not everyone is stoked about code reviews. While it is a good practice to have, it is often times seen as a hindrance to delivery and just plain annoying. But there is so much more to a code review than just nitpicking on someone else's code.
An opportunity to learn to be open-minded
The one reason why code review is being avoided is that it feels humiliating. But having someone else take a look at your code is the point of code review. The real problem is personality. Some don't take criticism well, some get too attached with their code, while others feel like their code is like gold.
A code review is a review about code. It has nothing to do with your capability and worth as a developer nor is it a measure of yourself against other developers. A review is not "Your code is terrible, therefore you're terrible." but "Your code is good, but we can help you make it better!". Keep calm, and refactor.
An opportunity to practice your reasoning skills
There are times you know you are right and you want to make things right. But a terrible presentation of your reasons for change might not get your point across. This means bad code slipping through causing technical debt or worse, a failure waiting to happen.
A code review can help, but you have to learn to be convincing with your review. You need to formulate your comments in a manner that strongly suggests things need to change. A set of coherent statements will always make more sense over blindly thrown ones.
An opportunity to teach others
How often do you encounter someone telling you that eval
and the Function
constructor in JavaScript should not be used? Probably a bunch of times. But did they explain exactly why it is avoided and/or provide you alternatives? Probably not. You probably just moved on with life with an unnecessary fear of eval
and the Function
constructor.
Code reviews should NOT promote FUD but instead facilitate learning. It should explain why the code is not ideal and what alternatives are available. In this manner, everyone leaves the review with new insights that they can share to others with great confidence.
Conclusion
There is so much more to a code review than just nitpicking on someone else's code. It is a fun exercise depending on who you are asking. I got sucked into several last week and spent 2 hours on them non-stop. I felt like the reviewer from hell or something. But it is all in the name of good code. Code review is the first line of defense against technical debt. Kill bad code before it becomes the zombie horde.
- Before
JSON.parse
, Douglas Crockford's JSON2 served as a shim and useseval
to parse JSON. - jQuery 1.x uses
Function
constructor to parse JSON as part of its AJAX functions. - Ractive uses the
Function
constructor to evaulate expressions instead of building its own parser. - The fact that I know the above is because I read open-source code once in a while. Humble brag.
- I do code reviews over at Code Review SE, especially when Stack Overflow gets boring.