How To Achieve The B.E.S.T. Code Review

4-Steps: Build, Explained, Seperated, And Tested

How To Achieve The B.E.S.T. Code Review

INTEGU - BEST-Code-Review-1

B.E.S.T. is the abbreviation of the four main requirements which should be fulfilled before taking a pull request onto a code review. Each part of the B.E.S.T. code review method, asks a question, which the developer should be able to answer with a strong and confident YES! The four steps of B.E.S.T., asks if the code have been BuildExplainedSeparated, and Tested.

 

 

 

In this post, I will be diving into each of the four questions and explain exactly what it means and how to fulfill it. Additionally, I will also come with some of my own tips and recommendations for how to better manage the code review process, from both the perspective of the Code Submitter and the Code Reviewer. Hopefully the content of this post, will be able to raise the quality of your pull request from an undocumented mess to an insightful and structure piece of work. Let’s make you colleagues compete to preform your code reviews!

INTEGU - First Code Review

Build?

Never ask for review before ensuring that the new changes can compile and that all unit tests are running. Did you change something major between the client and server communication? Better also check the end-to-end tests then. Don’t waste other people’s time before the code works.

Perhaps some people think that this is an unnecessary point and that it is of cause something any developer is aware of. However, I included this point because I will sometimes experience other developers (and sometimes even myself) being too quick on the trigger and request a review before the code has been build.

In my opinion, this is especially a problem when working on a small issue. You open the issue, change the relatively simple problem, turn to the closest developer and request a code review, thereby interrupting their workflow which we all known can sometimes be a pain in the %#& to get back into.

When then both of you sit in front of the screen, you quickly explain the issue and casually explain how you solved it. The reviewer takes a look at the code and asks if you have added a new unit test to ensure that the bug does not happen again. You then open the testing class, only to realize that you new change breaks several unit tests. – F***ing Great!

Now it has become a question of whether you need to change the unit tests or if the unit tests are ensuring that other functionalities are not broken by “hotshot” developers like you.

Either way, you need to go back and investigate the issue more thoroughly. Unfortunately, you already, stalled the solution to issue, with a premature code review and also interrupted your colleague’s workflow.

Good Job! – Next time, run “Build” first!

INTEGU - unit testing

Explained?

If you cannot explain it simply you have not understood it. don’t rely on the reviewer to be the expert. Explain the issue to your rubber duck (in my case a LEGO minion), and make sure that even a dummy would be able to understand it. When you got a grasp of the explanation and the concepts behind it, it is time to write the documentation.

How To Write Documentation:

  • Write individual explanations for each commit
  • Write the release note as the description of the pull request.
  • (if bug) Add information of how you solved the problem.
  • Explain how the pull request is structured and where to find important sections. “in the test.class line 10, I added a …”
  • Help the reviewer by adding the necessary documentation at the important code sections through the use of comments.
INTEGU - Rubber duck
When writing the explanation, it is important to use descriptive language and never make assumptions. Unless you know that the reviewer is already familiar with the specific topic or section of code, it should always be your job, as the code submitter, to explain it.

This type of in-depth explanation will also become helpful as time goes by, and you suddenly have to come back a modify the code. Luckily for the team, they can now rely on your explanation of the code instead of making you’re the “knowledge-sharing-bottleneck” of the project.

Separated?

Separation of Commits, into different categories (feature, bug fix, testing, refactoring) can bring you a long way in terms of conveying important details of a large pull request. Studies have even shown that reviews of more than 400 lines of code drastically reduce the chance of detecting mistakes.

INTEGU - Lines of code - review intensity
Let’s not lie to ourselves, I think we can all agree that the motivation to thoroughly review a pull request drops as the size of the submitted lines of code increase.

 

If the code is separated and small commits are made throughout the coding process, it will be much easier to review and understand. The reviewer will thereby also be able to determine error or mistakes much better, as they can focus on a single commit at a time, if necessary.

INTEGU - Lines of Code - Code review

Tested?

Testing, the art of not repeating yourself. If you are fixing a small annoying bug and don’t ever want to do that again, then write a test, which ensures that the code is never changed. Otherwise you are going to be right back to the same issue in six months.

“But writing tests takes time! Can’t I just only write a unit test, which checks the conditions that I have been working on?”

As a developer, your role is to ensure the quality of code, and writing tests are an important part of that.

If you look at the requirements set by the SIG (Software Improvement Group) in the book, Building Maintainable Software (Affliate Link), they argue that any method should have an equivalent amount of unit tests and branches (&&, ||, if-else-statements, switch). If you, as a developer want to write maintainable code, I suggest that you keep this in mind.

Naturally, I understand that this can sometimes be redundant, or maybe time is limited. However, I still emphasize that your primary responsibility lies with the quality of code and not time and budget management.

Code Submitter Tips

Don’t Rely On Your Presence

Code Review should not require your presence. If you need to explain it face-to-face, the review is likely going to cost more than the coding process. Additionally, you were not able to explain it in a simple way. Otherwise you could simply have written documentation on the pull request.

Taking code review face-to-face may be educational, since you can discuss with the other developer, but I would argue that you are learning more by being able to simply document the code change and then afterwards be able to argue pro and cons of the alternatives which the reviewer might recommend.

Additionally, face-to-face communication has the downside of no documentation.

Your Responsibility

Remove the responsibility from the reviewer and give it to yourself. If I am sitting with my mind stuck on some tricky issue and another developer on my team suddenly request a code review. I will personally set two requirement for the review:

  • The review must not take over 60 minutes, otherwise the issue was not explained well enough.
  • I should be able to do it on my own time. It is simple to schedule a code review, if it only requires a small window of time, is well documented and only requires the presence of the reviewer. However, if the review requires the presence of both the developer and reviewer, then we end up with a small task suddenly turning into a meeting .
INTEGU - Code-review-Reviewer-and-Submitter-expectation

Code Reviewer Tips

Review Checklists

When reviewing, you want to make sure that you have covered all relevant areas. Therefore, I would recommend start making a list of items which you will always be checking when you do a review.

In my opinion, it is best to create your own list and slowly add new points to it as your level of skill and knowledge increase.

I would argue that it is important to see the checklist as your personal list, since we do not want to enforce such processes on your SCRUM team. Always remember individuals over processes.

INTEGU - Review-Checklist

Keep Code And Language Clean

As a final note, I would like to point out that all of the abovementioned points are simply my opinion based on personal experience and study of other developer’s blogs and workflow.

Therefore, I would not advise you to start enforcing all of the recommendations onto your team. It is something which have to be build up over time and should be done in a manner which does not single out any team member.

Likewise, my last tip for the code reviewer, is to keep the tone of the communication nice and professional. Considering that you will often be reading comments on the pull request online, I would suggest to avoid phrasings like:

“This is unnecessary”

or

“Don’t inherit this class”

These sorts of comments aren’t constructive, and the code submitter only learns that they made a mistake, but not how to solve it. Instead try to be open and explain the problem.

“I can see what you are trying to do, but I think there is another way….”

or

“I think that it would be better to use this class because…”

….and lastly. If the pull request is well-written, do not hesitate to give the developer a nice acknowledgement. We can all grow from a supportive environment.

INTEGU - pull request approved

Recommended Reading

Videos

  • Top Signs of an Inexperienced Programmer, Techlead – Youtube

Blogs

  • Cracking the Code Review, Part 1 and Part 2 – Stephen R. – Blog
  • Best Practices for Code Review – Smartbear – Blog
  • Code Reviews: A Realistic Approach –
    Israel Gboluwaga – Blog

Books

  • Building Maintainable Software – Amazon

Other

  • IDEA Intellij: How to amend commits – Twitter
  • True story: one code review too many – Comic Strip
  • Fixing Unit Tests – Monkeyuser.com – Monkey User
  •  

About

Hi, I'm the Author

My name is Daniel H. Jacobsen and I’m a dedicated and highly motivated software developer with a masters engineering degree within the field of ICT. 

I have through many years of constantly learning and adapting to new challenges, gained a well-rounded understanding of what it takes to stay up to date with new technologies, tools and utilities. 

The purpose of this blog is to share both my learnings and knowledge with other likeminded developers as well as illustrating how these topics can be taught in a different and alternative manner.

If you like the idea of that, I would encourage you to sign up for the newsletter.

Cheers! 🍺

Didn't Find What You Were Looking For?

Generic selectors
Exact matches only
Search in title
Search in content
Search in posts
Search in pages
Scroll to Top
INTEGU - Cookie-consent

INTEGU uses cookies to personalize your experience and provide traceability for affiliate links. By using the website, you agree to these terms and conditions. To learn more see the privacy policy page.