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 Build, Explained, Separated, 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!
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!
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.
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.
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.
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.
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.
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.
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.