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.

Names

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.

Comments

Post a Comment

Keep it clean, professional and constructive. Everything else including ethnic or religious references will be removed.

Popular posts from this blog

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

Image
While writing tests for you code goes without saying, there is much to be said about code coverage of tests. How much of your code is actually covered by these tens of tests you just wrote? If you work in a TDD approach most probably you don't have such "mundane" questions. Of course you code is 100% covered. You did make sure every test failed before you added that line in your code to make it pass... and that conditional statement... or did you? While purists would argue there is no chance of no coverage I beg to differ. Let alone if you write your tests after the code is written... After such and such years and experience in the industry when we are nearing deadlines some things are thrown out the window. They shouldn't but they are. TL;DR tools help with coverage. In this case .Net Core Test Explorer with coverlet and Coverage Gutters . Photo by  Startup Stock Photos  from  Pexels Coverlet to the rescue As I blogged a while ago I have made myself a warm
Read more

NUnit Console Runner and NLog extension logging for your tests

Image
Have you experienced the frustration of tests running perfectly well on your local machine and then failing in CI? This all too familiar "It worked on my machine" comes to mind. You set off to investigate and after a while you realize you are too far into the rabbit hole between code and logs in Kibana and this investigation is getting you nowhere. Wouldn't it be nice if you could hijack the logger and print the logs in the test runner's console? TL;DR NLog can be extended easily and NUnit  Console Runner does not play well with simple console logs.   Photo by  Kaboompics .com  from  Pexels Enter Console logging In .Net in general it is not advisable to log to console in a multi-threaded environment, because the console is not thread safe. While logging simple strings may not cause any problems logging strings with interpolated variables may very well be sub optimal to say the least. Should you use any such debugging technique you are better off with a thread safe f
Read more

Visual Studio Code setup I use for C# development

Image
Visual Studio Code has been around a while. However most of the extensions or blog posts out there relate mainly to extensions that facilitate development in JavaScript environments such as Angular, React or NodeJs (to name a few) and even Rust! The IDE is getting great reviews as being fast to load and very responsive as opposed to its bloated and heavy bigger brother, Visual Studio. TL;DR I switched from Visual Studio for good Having worked on Visual Studio since version 4.2 I was used to it being heavy to load and at times even slow to respond. After experiencing VS Code with a little bit of NodeJs development I decided to try and make it my daily driver for .Net Core and C# development. I sought to find extensions to ease my day in the cubicle and see how comfortable will I be with it. I would give it a month on various tasks and workloads. VSCode showing off extensions The month became two months and then 3 months and here I am with a list of very handy (in my opinion)
Read more