Code Reviews are an important step in the software lifecycle, it helps to detect errors, propose improvements, distribute knowledge, mentor team members, and ensure teams follow standards. This should be conducted in a way that allows teams to make progress. For that sense comments should be grouped into 3 categories: required, recommended, and optional. This post will refer to naming elements in the code which fall into the “optional” category.
Many times during code reviews developers tend to overlook naming because that is “opinion only”, but as the project grows and new developers join, it becomes less maintainable, nobody captures what is done without a minimum half-hour of reading the code.
In this particular case, I am not referring to the use of only vowels for variables, like the classic ‘i’ that we have used for loops, which would have been many times more difficult to maintain. I am referring to the fact that even using complete English words for naming could still lead to confusion if not the right combination of meaningful words has been used.
My most recent experience on this matter: a month ago I was doing a code review and found a new entity that was including properties “type” and “typeId”, at a glance, I thought that the id was a numeric id for the enum “type”, then after asking the code review owner for clarification he explained that the “typeId” the property was meant for a subgroup of the type. I recommended renaming it for something else that makes it more self-explanatory but since this was not a blocker I added my approval after every other important comment was addressed. As in many other cases, recommendations ended up being ignored as others did not think they are important. A couple of weeks after that code was merged I had to deal with this new entity, as expected I forgot what this field was representing and had to spend 20 minutes reading through the code to understand how/why/when/where it was used to make sure my changes align with that. Just before feeling the need of writing this article, I had to live the same experience, which was enough motivation, I have been in this situation many times in my life already, from both sides of the problem. I would have named it “typeSubGroupId”, it may not be a very clear concept but will help others understand it is a subgroup without much trouble.
When is time to choose a name we should look at it as if we don’t have context at all and still understand the name we chose. It is hard to figure this out because when we are writing the code we have fresh knowledge of it, the weirder name we choose would still make sense during the first hour. Try to put aside the knowledge you have over this business logic or implementation details and choose a name that any new person could capture the meaning by just reading the name you chose.
When we see someone is not understanding a field/class/method’s name we should do our best to find a better name that describes better what it does or represent because sometimes from that moment somebody else will have the same problem.
Because I know this will keep happening, I am going to start paying more attention to any naming recommendation and putting more emphasis on my team to make sure everybody understands it is not just about opinion, it is also about our ability to maintain the code in the long run.
In summary, “naming” per se should not be considered a blocker for code reviews, but keep in mind that the more self-explanatory the code is, the easier will be for others (or even to who wrote it) to maintain, evolve and debug after some time.