Code Review +289 Code Review ‘A form of peer review in which code is incrementally inspected with an eye toward architectural concerns as the code is developed. Code review is a fantastic practice that every team should do anyway. Extending code reviews to include architectural concerns makes them even more powerful, Incrementally inspecting code as the architecture manifests helps fight architectural rot by keeping tabs on the system's evolution relative to the planned design. Reviews are also a great time to identify design inflection points that emerge during development. Such inflection points may require further analysis. Every review presents a potential coaching opportunity. Watch out for syn- chronization mismatches in mental models so you can fix them before real problems arise, Coach teammates on architectural principles as well as the specific architecture you're developing together. Benefits ‘« Keep architecture design at the forefront of every developer's mind. + Allow finer-grained details to emerge without losing a connection with coarser-grained architectural concerns. ‘+ Manage emergent details that can cause problems in the architecture, ‘Influence the detailed design as required. + Create teachable moments to grow the team's architecture design com- petence, Activity Timing Ongoing for the life of a project. An initial code review might take as little as 10 minutes with more time needed to resolve any identified issues. Participants ‘The author submits an artifact, in this case code, for review. The reviewer inspects the artifact and provides feedback. Many teams encourage multiple reviewers to participate.(Chapter 17. Actives to Evaluate Design Options * 290 Preparation and Materials + A prepared code artifact, patch, or pull request. Steps 1. Skim the change set to get a feel for the overall scope of changes. 2. Perform the code review as you normally would, focusing on detailed design, style issues, and defects. 3. Once the first pass review is complete, reflect on potential architectural implications of the change set. See the sample checklist on page 291 to get an idea of what you might look for during this part of the review. 4. Add comments related to your architectural concerns. If there are potential gaps in understanding, reference relevant resources. 5. After sharing your review with the submitter, follow up with that person directly. Architectural issues usually require more discussion, 6. Reflect on the results of the review. Were there issues that could have been avoided with more education or documentation? Is there an implied design decisions that should be explicitly stated? Add design tasks to the backlog to address ideas you think will bear fruit, such as improving a document or hosting an information session with the team, Depending on the tool or exact situation for your code review, these steps may need to be adapted, Guidelines and Hints * Use code review software that integrates with your version control and build system, + Reviews are often small. Watch out for thematic shifts in how the code evolves over time. Escalate the style of peer review to resolve issues quickly. For example, shift to pair programming or host a whiteboard jam on page 255 when an issue arises due to a lack of understanding, It's OK for the architecture to change as the system emerges. The primary goals of code review from the architect's perspective is to increase aware- ness of design decisions, monitor the implementation of the architecture, and guide change over time.Code Revew +291 + Code review is not a replacement for design evaluations. Use reviews to monitor architectural drift and learn how you can better serve your team, as an architect Example Code review checklists improve consistency and show teammates what to look for during a review. In addition to looking at detailed design concerns here are some architectural ideas you should look for when reviewing code: + Correctness—Are the changes consistent with the established patterns in the architecture? Are there pattern violations? Is there an opportunity to use an architecture pattern or refactor the code so that an intended pattern becomes more obvious? + Consistency—Look at the naming. Do the concepts at play make sense? Do any names surprise you? Are you able to form a mental model in your head of where the changes fit? How well does this jive with your expecta- Uons of what these changes would entail? « Testability—Are there clean unit tests included with the changes? Can the tests be run with every build? Is there an opportunity for the tests to be flaky or inconsistent? Are common patterns such as inversion of control used appropriately and correctly? + Modifiability/maintainability—Are there hard-coded constants or values that should be injected via configuration? What assumptions are baked into the code under review about what will change in the system? Can the code be made more flexible? Were any new dependencies introduced? Why were they introduced? Was it right to include them? ‘ Reliability—Are exceptions handled consistently? Are there opportunities for errors to propagate in unexpected or unhandled ways? Does the system attempt to retry when appropriate? Does the system fail fast when no recovery action can be attempted? How is error prevention (including from. human mistakes) built into the design? + Scalability—Does the code introduce potential for rampant memory use? Are the algorithms at least nominally efficient? Are thread-safe data structures used when appropriate?