Code Reviews are Required 5


Our employers, companies, and clients expect us to be professionals. They expect us to deliver quality products free of defects. How do we do that? Well, one way is to utilize code reviews.

Code reviews are a way of getting a second set of eyes on the code. When a developer is done with a particular feature, he or she may check-in a shelveset and request a review from another team member. The team member can provide a fresh perspective to the problem. They should be checking against the original user story. They should also be checking for any inconsistencies or potential problems in the code.

I’m primarily a .NET developer using Visual Studio and TFS. Formal code reviews can be completed easily within this environment. Reviews from within Visual Studio used to only be available through higher versions (Premium and above) of the product, but thankfully this feature has been added for Visual Studio 2015 Community and above.

 

Don’t just go through the motions

It’s important as a team to decide on what exactly should be reviewed. My personal preference is to review for both coding style and function.

If the naming conventions and coding style drastically differs from the rest of the application it may be quite jarring for the next poor soul that has to modify the code. When a developer is careless with his or her white space, or chooses poorly defined and meaningless variable and method names it’s questionable as to whether enough care was taken. A program should be consistent throughout and the reviewer should be aware of the style and provide direction if that style is not being followed.

If you’re the one doing the check-in and you’re looking for specific feedback, be explicit in your check-in comments. Follow up with the reviewer and work together to go over the items of concern. This is a great opportunity to pair-up and gain a better understanding during the review process.

 

Keep it short

Reviews should be short. In an effort to keep effected changes limited, the associated check-ins should be small. This should help keep the feedback loop fast. If you’re asking someone to review a major feature with hundreds of files and thousands of lines of code, that’s just too daunting a task to undertake in a single review. In the Agile spirit, keep the feedback loop small. Keep your changes narrow in scope and manageable.

 

Check-in against tasks and stories

For historical purposes it’s important that you check-in against Tasks and Stories. If you’re using TFS or VSTS for your code repository you can easily associate your changeset with a Task, Story, or Bug. I like to associate with a both a Task and it’s parent Story so that anyone looking for changes associated with a particular item can get the whole picture of what was involved with a particular change.

 

Be constructive

For reviewers as well as requesters it’s important to be courteous, constructive, and open to criticism. Remember, none of the comments should be personal and should be taken as such. The objective of the code review is to improve the quality of the product being delivered to the customer.

If, as a reviewer, you find yourself providing tons of feedback during a review, or if you’re overwhelmed by the amount of changes associated with the request, it may be best to seek out the requester. Spend some time with the individual and guide them to better practices associated with the code review process. Instruct them on better coding practices, if that’s where the majority of your comments were focused.

 

Be open

As a requester, take this opportunity to learn and grow as a developer. I’ve learned quite a bit from reviewers who were more knowledgeable than I with regards to a particular language, pattern, or application. Don’t be afraid to admit you don’t know something. Be willing to accept help when it is given. After all, the goal is to develop and deliver a better product.

 

Provide value

Above all else, make sure the process is providing value. You may need to adjust the code review process to fit your team. Evaluate the process and see if there are any improvements that you could make. Make sure there is benefit to you, your team, your company, and your clients.

 

###

 

What other practices do you find important with regards to Code Reviews? What other processes has your team put into place?

 

 

A Microsoft MVP, John has been a professional developer since 1999. He has focused primarily on web technologies and has experience with everything from PHP to C# to ReactJS to SignalR. Clean code and professionalism are particularly important to him, as well as mentoring and teaching others what he has learned along the way.

Please Consider Sharing This Post:

Facebooktwittergoogle_plusredditlinkedinFacebooktwittergoogle_plusredditlinkedin
  • John Voris

    Thanks. This was a good review of the Peer Code Review process.
    I hope you will do a follow up write-up on Pair Programming, where the Code Review happens in Real Time and which facilitates Unit Tests, discussing Shop Standards and readability, and above all else, Quality Code.

    • Good idea, John. I’ll definitely be continuing on the theme. Thanks for the suggestions and encouragement!

  • Rajat Agrawal

    Sometimes even before you do code reviews you need to make the DB schema and perform a schema review on it to check for all the possible constraints like referencial, check, keys etc., normalization levels, naming, columns order, data type are in place or not. So that you can proceed to build your code/module/component over the new schema. For schema reviews the DDL statements in a notepad file or the SQL server schema diagram should suffice.

  • Zam

    Hello!
    Let me say that I’m a supporter of Code Reviews but some arguments that I’ve heard around are not very convincing.
    First, Reviews will not improve the quality when 2 developers have completely opposite opinions. It will usually require a third opinion and maybe they will not end on the big Q result.
    You may say, well, “you have to be open to other’s criticism”. I agree that one have to be constructive, open and add value… but it is very subjective. Let’s say for example that some evil tool like Stylecop tells you that you must add comments to every single property on a class. One programmer could think that this comment violates DRY (because it’s saying exactly the same to what any programmer can read by just looking at the code), and the other thinks it is very necessary in order to generate documentation.
    Another thing that must be thought twice is the pseudo-fact about the next poor programmer that would look into the code. It depends. Coding Style is good, but also Personal Style has some value and information to add. In general, it is good to share some guidelines, but it is counter-productive to force the style. It must evolve with the particulars. It is very valuable when you look at long-period developments, the style itself can add some information like: “ahhh, it was written when the teams were not joined yet” or “ahhh… Bobby was here”. Yes, you can also look at Source Control history… change by change.

    • Hi Zam, thanks for the comments. Excellent points. Change, improve, evolve. Also, comments are evil. 😉