Code reviews and naming conventions (or when the obsession for naming conventions misses the point of the review)

The Past

It has been well over 6 years since I had my code reviewed by my peers. As a development team leader I had reviewed the code of my developers, however most of the time they didn't review mine. This was due to either them being juniors or mostly uninterested. I had asked for reviewing and I always got a "A OK" answer. The organizations I worked for did not really enforce this either. They were busy releasing to customers and to damnation with technical debts (or tests but this is another story). TL;DR When in Rome do as Romans do

What to look for in a code review

My take on what to look for in a code review is quite simple.

Abide to the overall design agreed

Be more strict to juniors as they got a more detailed design hand-off. Be more lenient to reasonable changes by seniors or seasoned developers. Ask for the logic behind the change and maybe debate the change to a degree of common ground and understanding.

Find bugs 

Yes there may be tests and a developer may have been checking the code extensively. However we are bugs that create developers... 

Abide to coding conventions

This is a simple one right? Make sure variables, the camel case of methods or the grouping of methods by visibility abides to that 1000 line excel file someone created a few years ago. Make sure some naming patterns are kept, for example add the word Implementation at the end of a class implementing some interface.

Check for typing errors

"Elementary my dear Watson". If a variable name has a typo it will make it hard for the rest of the team to locate it.

General code correctness

Make sure proper design, best practices and coding patterns are used. Make sure anti-patterns are not. Clean technical debt. Remove  dead code and unnecessary comments or commented out code.


Finally the reason we are gathered here today. There are 10 kinds of people when it comes to names. Those who spend 0-30 seconds on them and those who can spend days on them. I personaly never gave them much thought unless they were way off the beaten path or their meaning made absolute no sense.

The Present

Last summer I decided to make a change. Have fun again. See and design more systems, meet new people all the time but first and foremost learn a new thing every week if not every day. I joined a company of super professional and knowledgeable consultants, who also enjoy learning from one another all the time.
As such one meets all sorts of development methodologies. Some enforce religiously unit tests and code reviews (Good! Good! Emperor Palpatine, Return of the Jedi)

Names in code reviews

When designing a system with your team you may choose temporary names to call certain aspects of your system. Some times these names have synonyms and sometimes these names, however temporary, stick. If you decide to change the names, no matter what the reason may be, make sure to clearly communicate the change to your team members. If the change was not clearly communicated, when code reviewing just make simple request to make some name changing. Do not however put all or most of the weight of the review on names. Put that issue aside and concentrate on the more important things.
This portrays a trait me and some friends of mine have spotted in many team leaders in our industry. The need to stick to the minor meaningless details and (dis)miss all the rest.

Conclusion and insights

Stick to the important things first when code reviewing. If the names make an overall sense live with it. Teach your people how to code correctly. The names are subjective. Totally subjective. Enforcing the name  just enforces one's mindset on another. If this becomes a thing of habit down the road all the developer will remember is the naming OCD of the team leader. And the team leader will remember the developer as unable to comply.
And finally when being a consultant flow with the client. Live the sudden changes and quirks of each client and learn to be flexible between different clients.


Popular posts from this blog

NUnit Console Runner and NLog extension logging for your tests

Tests code coverage in Visual Studio Code with C# and .Net Core

Applying an internal forums system in the company culture - thoughts