Why You Should Review Your Own Code And How To Review It Effectively
October 28, 2019
Before we start, I want to briefly discuss what a code review is. Essentially, a code review is when someone, most likely another person on your team/project, looks at the code you wrote for a specific feature and checks it for consistency, errors, styling, and more. Sometimes the person reviewing the code will also test the code locally in their own development environment to ensure all the requirements are met. I personally think that for any non-trivial code changes the reviewer should at the very least do basic testing of the feature to ensure that nothing was missed by the original developer. You would be surprised how often I review code that doesn’t even work. I actually think this step is so important, that the very first thing I do when reviewing code is download the code and test it based on the exact requirements of the feature. I do this before I even look at any of the code, because I want to test everything as if I was a user since that is how people will use the feature. If I look at the code before doing my testing then I will be biased since I know how the code works and may use the feature only in the exact way the code intends the feature to work.
You may think that this test is useless if you are reviewing your own code, since you wrote the code and have tested it many times, but it is very easy to get lost in writing code and forget to implement the feature exactly as it is specified. Sometimes when I am coding a feature I will read the feature and start coding it, but never actually go back and re-read the feature to ensure I am following it exactly. By reviewing my own code, I force myself to re-read what the feature should do and test my code on exactly the specifications of the feature. I generally find it best to do this review of your own code at least one day after you wrote the code. This removes most of the bias I talked about in the last paragraph and makes your testing more focused on how the user will use the feature.
Now after I have finished testing the code, the next thing to do is start looking at the actual code. This is where many people dive right in and start making comments on things to change, but I caution against this. Instead the best thing to do is first read all the code from beginning to end. You want to ensure you have a solid understanding of what the code is doing. This doesn’t have to be a perfect understanding, but you should have a general idea of what is happening in the code. If in reading any of the code you find sections that are confusing or do not make immediate sense then this is where you should leave your first comments. Difficult to understand code is the quickest way to build a code base that is painful to work in and difficult to add to, so fixing confusing code should be a top priority. If you truly have no idea what is going on in the code, leave a comment saying so, but if the code is just confusing then try to leave a suggestion on how to clean up the code and make it more understandable. A code reviewer’s job is not to tell the other person where they messed up, but instead their job is to help the other person write better code. This is why suggestions are always better than comments saying something is bad or wrong.
After reading all the code and leaving comments on difficult to understand code, it is now time to start reviewing the code for correctness. This involves two steps. The first step is reading the code and checking for business logic errors or code mistakes. This could be something as simple as forgetting to check for null when an object could be null, or a more complex business mistake like accidentally comparing emails using case sensitive comparisons. The second step is re-testing the application manually, but specifically testing for the edge cases. One common example would be to test what a list looks like when there are no items in it, or how a form handles empty inputs when you submit. By combining your knowledge of the code with the same level of testing from the first step you can find new bugs or errors that would be hard to uncover otherwise. If you do find bugs/errors in this step it is generally a good idea to suggest the creation of an automated test for the edge case that is broken.
Most people that do code reviews stop here, but they are missing the most important step of any code review which is code quality. After checking to ensure the code works properly and is easy to read, the next thing that needs to be checked for is the quality of the code. One easy thing to check for is whether or not the core business logic of the feature is covered by automated tests. If at the very minimum the most important business logic of a feature is not tested then you need to leave comments suggesting which parts of the code you feel should be tested. Automated testing is one of the most important ways to ensure an application is working properly, so making sure to check for good test coverage in code reviews is incredibly important.
After checking to ensure test coverage is adequate, the next thing to check for is the structure of the code. When many people think of code structure they think of tabs vs spaces, how much indentation to use, how long a line of code should be, etc., but these things should all be taken care of by a linter. Reviewing code structure is all about the overall architecture of the code itself. There is some overlap between reviewing the code structure and reviewing the complexity of the code, but in general code complexity is reviewed on a smaller scale, and code structure is the overall large scale of the entire project/feature. This is the hardest thing to check when reviewing code since it is difficult to predict how the current code structure will impact future features. The biggest thing to look for when reviewing code structure is consistency. If the code being reviewed is consistent in structure and design to similar existing code then that is a good sign that the code is well structured. When the code starts to deviate from the norm you should pay extra attention to figure out why the deviation occurred, and if it is something that should be applied to the rest of the code base as well. Another great way to check the structure of the code is with a gut check. If something feels off or wrong or bad about the code then that is generally a sign that the structure of the code could use improvement. This is something that seems subjective, but more times than not a gut feeling of something being off is an accurate indicator of bad quality code.
And with that final step you have successfully completed a quality code review. To recap, the first step is to test the feature. After the feature is completely tested then you want to read all the code changes to gain an understanding of the code and check for any overly complex code. Next comes the validation portion of code review where you check the code for errors and incorrect business logic. Lastly, you want to review the code structure and quality to ensure the new code does not make the overall code base more difficult to work with.