The normal procedure for carrying out a code review is to make sure your code compiles and meets the requirements set. It should also pass all unit tests and end-to-end tests. Once you are confident that you are able to compile, test, and run your code successfully, then it is checked in to the current working branch. Once checked in, you will then issue a pull request.

A peer reviewer will then review your code and share comments and feedback. If your code passes the code review, your code review is completed and you can then merge your working branch into the main trunk. Otherwise, the peer review will be rejected, and you will be required to review your work and address the issues raised in the comments provided by your reviewer.

The following diagram shows the peer code review process:

Preparing for a code review can be a right royal pain at times, but it does work for better overall quality of code that is easy to read and maintain. It is definitely a worthwhile practice that teams of developers should carry out as standard coding procedures. This is an important step in the code review process, as perfecting this step can save the reviewer considerable time and energy in performing the review.

Here are some standard points to keep in mind when preparing your code for review:

• Always keep the code review in mind: When beginning any programming, you should have the code review in mind. So keep your code small. If possible, limit your code to one feature.
• Make sure that all your tests pass even if your code builds: If your code builds but you have failing tests, then deal immediately with what's causing those tests to fail. Then, when the tests pass as expected, you can move on. It is important to make sure that all unit tests are passed, and that end-to-end testing passes all tests. It is important that all testing is complete and gets the green light, since releasing code that works but was a test fail could result in some very unhappy customers when the code goes to production.

• Remember YAGNI: As you code, make sure to only add code that is necessary to meet the requirement or feature you are working on. If you don't need it yet, then don't code it. Only add code when it is needed and not before.
• Check for duplicate code: If your code must be object-oriented and be DRY and SOLID, then review your own code to see whether it contains any procedural or duplicate code. Should it do so, take the time to refactor it so that it is object-oriented, DRY, and SOLID.
• Use static analyzers: Static code analyzers that have been configured to enforce your company's best practices will check your code and highlight any issues that are encountered. Make sure that you do not ignore information and warnings. These could cause you issues further down the line.

When leading code reviews, it is important to have the right peoplepresent. The people who will be in attendance at the peer code review will be agreed upon with the project manager. The programmer(s) responsible for submitting the code for review will be present at the code review unless they work remotely. In the case of remote working, the reviewer will review the code and either accept the pull request, decline the pull request, or send the developer some questions to be answered before taking any further action.

A suitable lead for a code review should possess the following skills and knowledge:

• Be a technical authority: The person leading the code review should be a technical authority that understands the company's coding guidelines and software development methodologies. It is also important that they have a good overall understanding of the software under review.
• Have good soft skills: As the leader of the code review, the person must be a warm and encouraging individual who is able to provide constructive feedback. It is important that the person reviewing the programmer's code has good soft skills so that there is no conflict between the reviewer and the person whose code is being reviewed.
• Not be overly critical: The leader of the code review must not be over-critical and must be able to explain their critique of the programmer's code. It is useful if the leader has been exposed to different programming styles, and can view the code objectively to ensure that it meets the project's requirements.

In my experience, peer code reviews are always carried out on pull requests in the version control tool being used by the team. A programmer will submit the code to version control and then issue a pull request. The peer code reviewer will then review the code in the pull request. Constructive feedback will be provided in the form of comments that will be attached to the pull request. If there are problems with the pull request, then the reviewer will reject the change request and comment on specific issues that need to be addressed by the programmer. If the code review is successful, then the reviewer may add a comment providing positive feedback, merge the pull request, and close it.

Programmers will need to note any comments made by the reviewer and take them on board. If the code needs to be resubmitted, then the programmer will need to ensure that all the reviewer's comments have been addressed prior to resubmitting.

It is a good idea to keep code reviews short, and to not review too many lines at any one time.

Since a code review normally starts with a pull request, we will look at issuing a pull request followed by responding to a pull request.

Issuing a pull request

When you have finished coding and you are confident in the quality of your code and that it builds, you are able to then push or check in your changes, depending on what source control system you use. When your code has been pushed, you can then issue a pull request. When a pull request is raised, other people that are interested in the code are notified and able to review your changes. These changes can then be discussed and comments made regarding any potential changes that you need to make. In essence, your pushing to your source control repository and issuing a pull request is what kick-starts the peer code review process.

To issue a pull request, all you have to do (once you've checked your code in or pushed it) is click on the Pull requests tab of your version control. There will then be a button you can click on – New pull request. This will add your pull request to a queue to be picked up by the relevant reviewers.

In the following screenshots, we will see the process of requesting and fulfilling a pull request via GitHub:

1. On your GitHub project page, click on the Pull requests tab:

2. Then, click on the New pull request button. This will display the Comparing changes page:

3. If you are happy, then click on the Create pull request button to start the pull request. You will then be presented with the Open a pull request screen:

4. Write your comment regarding the pull request. Provide all the necessary information for the code reviewer, but keep it brief and to the point. Useful comments include identification of what changes have been made. Modify the Reviewers, Assignees, Labels, Projects, and Milestones fields as necessary. Then, once you are happy with the pull request details, click on the Create pull request button to create the pull request. Your code will now be ready to be reviewed by your peers.

Responding to a pull request

Since the reviewer is responsible for reviewing pull requests prior to merges of branches, we would do well to look at responding to pull requests:

1. Start by cloning a copy of the code under review.
2. Review the comments and changes in the pull request.
3. Check that there are no conflicts with the base branch. If there are, then you will have to reject the pull request with the necessary comments. Otherwise, you can review the changes, make sure the code builds without errors, and make sure there are no compilation warnings. At this stage, you will also look out for code smells and any potential bugs. You will also check that the tests build, run, are correct, and provide good test coverage of the feature to be merged. Make any comments necessary and reject the pull request unless you are satisfied. When satisfied, you can add your comments and merge the pull request by clicking on the Merge pull request button, as shown here:

4. Now, confirm the merge by entering a comment and clicking on the Confirm merge button:

5. Once the pull request has been merged and the pull request closed, the branch can be deleted by clicking on the Delete branch button, as can be seen in the following screenshot:

In the previous section, you saw how the reviewee raises a pull request to have their code peer-reviewed before it is merged. And in this section, you have seen how to review a pull request and complete it as part of a code review. Now, we will look at what to review in a peer code review when responding to a pull request.

Effects of feedback on reviewees

When performing a code review of your peer's code, you must also take into consideration the fact that feedback can be positive or negative. Negative feedback does not provide specific details about the problem. The reviewer focuses on the reviewee and not on the problem. Suggestions for improving the code are not offered to the reviewee by the reviewer, and the reviewer's feedback is aimed at hurting the reviewee.

Such negative feedback received by the reviewee offends them. This has a negative impact and can cause them to start doubting themselves. A lack of motivation then develops within the reviewee and this can negatively impact the team, as work is not done on time or to the required level. The bad feelings between the reviewer and the reviewee will also be felt by the team, and an oppressive atmosphere that negatively impacts everyone on the team can ensue. This can lead to other colleagues becoming demotivated, and the overall project can end up suffering as a result.

In the end, it gets to the point where the reviewee has had enough and leaves for a new position somewhere else to get away from it all. The project then suffers time-wise and even financially, as time and money will need to be spent on finding a replacement. Whoever is found to fill the position then has to be trained upon the system and the working procedures and guidelines. The following diagram shows negative feedback from the reviewer toward the reviewee:

Conversely, positive feedback from the reviewer to the reviewee has the opposite effect. When the reviewer provides positive feedback to the reviewee, they focus on the problem and not on the person. They explain why the code submitted is not good, along with the problems it can cause. The reviewer will then suggest to the reviewee ways in which the code can be improved. The feedback provided by the reviewer is only done to improve the quality of the code submitted by the reviewee.

When the reviewee receives the positive (constructive) feedback, they respond in a positive manner. They take on board the reviewer's comments and respond in the appropriate manner by answering any questions, asking any relevant questions themselves, and the code is then updated, based on the reviewer's feedback. The amended code is then resubmitted for review and acceptance. This has a positive impact on the team as the atmosphere remains a positive one, and work is done on time and to the required quality. The following diagram shows the results of positive feedback on the reviewee from the reviewer:

The point to remember is that your feedback can be constructive or destructive. Your aim as a reviewer is to be constructive and not destructive. A happy team is a productive team. A demoralized team is not productive and is damaging to the project. So, always strive to maintain a happy team through positive feedback.

A technique for positive criticism is the feedback sandwich technique. You start with praise on the good points, then you provide constructive criticism, and then you finish with further praise. This technique can be very useful if you have members on the team that doesn't react well to any form of criticism. Your soft skills in dealing with people are just as important as your software skills in delivering quality code. Don't forget that!

We will now move on to look at what we should review.

There are different aspects of the code that have to be considered when reviewing it. Primarily, the code being reviewed should only be the code that was modified by the programmer and submitted for review. That's why you should aim to make small submissions often. Small amounts of code are much easier to review and comment on.

Let's go through different aspects a code reviewer should assess for a complete and thorough review.

Company's coding guidelines and business requirement(s)

All code being reviewed should be checked against the company's coding guidelines and the business requirement(s) the code is addressing. All new code should adhere to the latest coding standards and best practices employed by the company.

There are different types of business requirements. These requirements include those of the business and the user/stakeholder as well as functional and implementation requirements. Regardless of the type of requirement the code is addressing, it must be fully checked for correctness in meeting requirements.

For example, if the user/stakeholder requirement states that as a user, I want to add a new customer account, does the code under review meet all the conditions set out in this requirement? If the company's coding guidelines stipulate that all code must include unit tests that test the normal flow and exceptional cases, then have all the required tests been implemented? If the answer to any of these questions is no, then the code must be commented on, the comments addressed by the programmer, and the code resubmitted.

Naming conventions

The code should be checked to see whether the naming conventions have been followed for the various code constructs, such as classes, interfaces, member variables, local variables, enumerations, and methods. Nobody likes cryptic names that are hard to decipher, especially if the code base is large.

Here are a couple of questions that a reviewer should ask:

• Are the names long enough to be human-readable and understandable?
• Are they meaningful in relation to the intent of the code, but short enough to not irritate other programmers?

As the reviewer, you must be able to read the code and understand it. If the code is difficult to read and understand, then it really needs to be refactored before being merged.

Formatting

Formatting goes a long way to making code easy to understand. Namespaces, braces, and indentation should be employed according to the guidelines, and the start and end of code blocks should be easily identifiable.

Again, here is a set of questions a reviewer should consider asking in their review:

• Is code to be indented using spaces or tabs?
• Has the correct amount of white space been employed?
• Are there any lines of code that are too long that should be spread over multiple lines?
• What about line breaks?
• Following the style guidelines, is there only one statement per line? Is there only one declaration per line?
• Are continuation lines correctly indented using one tab stop?
• Are methods separated by one line?
• Are multiple clauses that make up a single expression separated by parentheses?
• Are classes and methods clean and small, and do they only do the work they are meant to do?

Testing

Tests must be understandable and cover a good subset of use cases. They must cover the normal paths of execution and exceptional use cases. When it comes to testing the code, the reviewer should check for the following:

• Has the programmer provided tests for all the code?
• Is there any code that is untested?
• Do all the tests work?
• Do any of the tests fail?
• Is there adequate documentation of the code, including comments, documentation comments, tests, and product documentation?
• Do you see anything that stands out that, even if it compiles and works in isolation, could cause bugs when integrated into the system?
• Is the code well documented to aid maintenance and support?

Let's see how the process goes:

Untested code has the potential to raise unexpected exceptions during testing and production. But just as bad as code that is not tested are tests that are not correct. This can lead to bugs that are hard to diagnose, can be annoying for the customer, and make more work for you further down the line. Bugs are technical debt and looked upon negatively by the business. Moreover, you may have written the code, but others may have to read it as they maintain and extend the project. It is always a good idea to provide some documentation for your colleagues.

Now, concerning the customer, how are they going to know where your features are and how to use them? Good documentation that is user-friendly is a good idea. And remember, not all your users may be technically savvy. So, cater to the less technical person that may need handholding, but do it without being patronizing.

As a technical authority reviewing the code, do you detect any code smells that may become a problem? If so, then you must flag, comment, and reject the pull request and get the programmer to resubmit their work.

As a reviewer, you should check that those exceptions are not used to control the program flow and that any errors raised have meaningful messages that are helpful to developers and to the customers who will receive them.

Architectural guidelines and design patterns

The new code must be checked to see whether it conforms to the architectural guidelines for the project. The code should follow any coding paradigms that the company employs, such as SOLID, DRY, YAGNI, and OOP. In addition, where possible, the code should employ suitable design patterns.

This is where the Gang-of-Four (GoF) patterns come into play. The GOF comprises four authors of a C++ book called Design Patterns: Elements of Reusable Object-Oriented Software. The authors were Erich Gamma, Richard Helm, Ralph Johnson, and John Vlissides.

Today, their design patterns are heavily used in most, if not all, object-oriented programming languages. Packt has books that cover design patterns, including .NET Design Patterns, by Praseen Pai and Shine Xavier. Here is a really good resource that I recommend that you visit: https://www.dofactory.com/net/design-patterns. The site covers each of the GoF patterns and provides the definition, UML class diagram, participants, structural code, and some real-world code for the patterns.

GoF patterns consist of creational, structural, and behavioral design patterns. Creational design patterns include Abstract Factory, Builder, Factory Method, Prototype, and Singleton. Structural design patterns include Adapter, Bridge, Composite, Decorator, Façade, Flyweight, and Proxy. Behavioral design patterns include Chain of Responsibility, Command, Interpreter, Iterator, Mediator, Memento, Observer, State, Strategy, Template Method, and Visitor.

The code should also be properly organized and placed in the correct namespace and module. Check the code also to see whether it is too simplistic or over-engineered.

Performance and security

Other things that may need to be considered include performance and security:

• How well does the code perform?
• Are there any bottlenecks that need to be addressed?
• Is the code programmed in such a way to protect against SQL injection attacks and denial-of-service attacks?
• Is code properly validated to keep the data clean so that only valid data gets stored in the database?
• Have you checked the user interface, documentation, and error messages for spelling mistakes?
• Have you encountered any magic numbers or hard coded values?
• Is the configuration data correct?
• Have any secrets accidentally been checked in?

A comprehensive code review will encompass all of the preceding aspects and their respective review parameters. But let's find out when it is actually the right time to even be performing a code review.

Code reviews should take place when the development is complete and before the programmer of the code passes the code on to the QA department. Before any code is checked into version control, all the code should build and run without errors, warnings, or information. You can ensure this by doing the following:

• You should run static code analysis on your programs to see whether any issues are raised. If you receive any errors, warnings, or information, then address each point raised. Do not ignore them as they can cause problems further down the line. You can access the Code Analysis configuration dialog on the Code Analysis page of the Visual Studio 2019 Project Properties tab. Right-click on your project and select Properties | Code Analysis.
• You should also make sure that all your tests run successfully, and you should aim to have all your new code to be fully covered by normal and exceptional use cases that test the correctness of your code against the specification you are working on.
• If you employ a continuous development software practice within your place of work that integrates your code into a larger system, then you need to make sure that the system integration is successful and that all tests run without failing. If any errors are encountered, then you must fix them before you go any further.

When your code is complete, fully documented, and your tests work, and your system integration all works without any issues, then that is the best time to undergo a peer code review. Once you have reached the point that your peer code review is approved, your code can then be passed on to the QA department. The following diagram shows the Software Development Life Cycle (SDLC) from the development of the code through to the end of the life of the code:

The programmer codes the software as per specifications. They submit the source code to the version control repository and issue a pull request. The request is reviewed. If the request fails, then the request is rejected with comments. If the code review passes, then the code is deployed to the QA team that carry out their own internal testing. Any bugs found are raised for the developers to fix. If the internal testing passes QA, then it is deployed into User Acceptance Testing (UAT).

If UAT fails, then bugs are raised with the DevOps team, who could be developers or infrastructure. If UAT passes QA, then it is deployed to staging. Staging is the team responsible for deploying the product in the production environment. When the software is in the hands of the customer, they raise a bug report if they encounter any bugs. Developers then work on fixing the customer's bugs, and the process is restarted. Once the product reaches the end of its life, it is retired from service.

It is worth remembering that code reviews are aimed at the overall quality of code in keeping with the company's guidelines. Feedback, therefore, should be constructive and not used as an excuse to put down or embarrass a fellow colleague. Similarly, reviewer feedback should not be taken personally and responses to the reviewer should focus on suitable action and explanation.

The following diagram shows the process of issuing a Pull Request (PR), performing a code review, and either accepting or rejecting the PR:

Providing feedback as a reviewer

Workplace bullying can be a problem, and programming environments are not immune. Nobody likes a cocky programmer who thinks they are big. So, it is important that the reviewer has good soft skills and is very diplomatic. Bear in mind that some people can easily be offended and take things the wrong way. So know who you are dealing with and how they are likely to respond; this will help you choose your approach and your words carefully.

As the peer code reviewer, you will be responsible for understanding the requirements and making sure the code meets that requirement. So, look for the answers to these questions:

• Are you able to read and understand the code?
• Can you see any potential bugs?
• Have any trade-offs been made?
• If so, why were the trade-offs made?
• Do the trade-offs incur any technical debt that will need to be factored into the project further down the line?

Once your review is complete, you will have three categories of feedback to choose from: positive, optional, and critical. With positive feedback, you can provide commendations on what the programmer has done really well. This is a good way to bolster morale as it can often run low in programming teams. Optional feedback can be very useful in helping computer programmers to hone their programming skills in line with the company guidelines, and they can work to improve the overall wellbeing of the software being developed.

Finally, we have critical feedback. Critical feedback is necessary for any problems that have been identified and must be addressed before the code can be accepted and passed on to the QA department. This is the feedback where you will need to choose your words carefully to avoid offending anyone. It is important that your critical comments address the specific issue being raised with valid reasons to support the feedback.

Responding to feedback as a reviewee

As the reviewee programmer, you must effectively communicate the background of your code to your reviewer. You can help them by making small commits. Small amounts of code are much easier to review than large amounts of code. The more code being reviewed, the easier it is for things to be missed and slip through the net. While you are waiting for your code to be reviewed, you must not make any further changes to it.

As you can guess, you will receive either positive, optional, or critical feedback from the reviewer. The positive feedback works to boost your confidence in the project as well as your morale. Build upon it and continue with your good practices. You may choose to act or not upon optional feedback, but it's always a good idea to talk it through with your reviewer.

For critical feedback, you must take it seriously and act upon it as this feedback is imperative for the very success of the project. It is very important that you handle critical feedback in a polite and professional manner. Don't allow yourself to be offended by any comments from your reviewer; they are not meant to be personal. This is especially important for new programmers, and programmers who lack confidence.

As soon as you receive your reviewer's feedback, act upon it, and make sure that you discuss it with them as necessary.

In this chapter, we have discussed the importance of performing code reviews and the complete process of getting code ready for review and responding to reviewer comments as the programmer, along with how to lead a code review and what to look for when performing a review as the code reviewer. It can be seen that there are clearly two roles in a peer code review. These are the reviewer and the reviewee. The reviewer is the person performing the code review, and the reviewee is the person whose code is being reviewed.

You have also seen how you, as a reviewer, can categorize your feedback and why soft skills are important when providing feedback to fellow programmers. And as a reviewee whose code is being scrutinized, you have seen how important it is to build upon positive and optional feedback and how important it is to act upon critical feedback.

By now, you should have a good understanding of why it is important to conduct regular code reviews, and why they should be done before the code is passed on to the QA department. Peer code reviews do take time and can be uncomfortable for both the reviewer and reviewee. But in the long run, they work toward a high-quality product that is easy to extend and maintain, and they lead to better code reuse as well.

In the next chapter, we will be looking at how to write clean classes, objects, and data structures. You will see how we can organize our classes, ensure our classes only have one responsibility, and comment on our classes in order to assist with documentation generation. We will then look at cohesion and coupling, designing for change, and the Law of Demeter. Then, we will look at immutable objects and data structures, hiding data, and exposing methods in objects, before finally looking at data structures.

1. What are the two roles involved in a peer code review?
2. Who agrees on the people that will be involved in the peer code review?
3. How can you save your reviewer time and effort prior to requesting a peer code review?
4. When reviewing code, what kinds of things must you look out for?
5. What are the three categories of feedback?

• https://docs.microsoft.com/en-us/visualstudio/code-quality/?view=vs-2019: This documentation by Microsoft provides information on the different tools available to help you analyze and improve the quality and maintainability of your code.
• https://en.wikipedia.org/wiki/Code_review: There are many useful links on this page to further your knowledge of code reviews and their value to your business.
• https://springframework.guru/gang-of-four-design-patterns/: Gang-of-Four design patterns book.
• https://www.packtpub.com/application-development/net-design-patterns: .NET Design Patterns, by Praseed Pai and Shine Xavier.
• https://help.github.com/en: GitHub's help page.