From the course: Inclusive Tech: Conducting Humane Code Reviews

Goal 1: Catch design flaws

From the course: Inclusive Tech: Conducting Humane Code Reviews

Start my 1-month free trial

Goal 1: Catch design flaws

- [Narrator] When thinking of why we do code reviews, the first and foremost reason you probably thought of is to catch design flaws, and you'd be right. These are the most important types of mistakes to catch in a code review. Design flaws could be as obvious as unused functions in code, or they could be less obvious like bugs in the overall architecture of your code. Simply put design flaws are things that degrade the overall state of your code base. So how do you find design flaws in code? You can start by asking yourself these questions as you carefully review the code changes about to be merged. Things like does this code integrate well with the rest of your architecture? For example, if your application uses MVC architecture that you see a controller type logic in a view you'll want to point that out in the code review. Code that goes against your existing architecture. For example, if your application uses MVC architecture but you see controller type logic in a view, you'll want to point that out in the code review. Code that goes against your existing architecture, assuming that you aren't refactoring it, should immediately be brought to the developers' attention in a code review comment. Other things like do the interactions between various portions of changed code makes sense? This tends to happen when you build upon existing code and functionality. It's even more prevalent when you do large refactors. These kinds of changes can disrupt the existing flows of your application and are another extremely important design flaw to look for during code reviews. Next, are there any race conditions? When you deal with concurrent programming, keep an eye out for any potential threads or processes that may compete for the same resource. By having two threads, try to access this at the same time you can get weird, only happens in certain situations type of bugs that you don't want affecting your application. You may even get unexpected yet different results each time you try to run that part of the code. Another thing, does the logic seems sound and robust enough for edge cases? Your coach certainly handle the happy paths, AKA, the developers intended use cases and workflows for everyone that will use their application. But it's also important to anticipate other scenarios once in a blue moon scenarios, scenarios that don't assume network connectivity, a specific device size and so much more. Looking for these assurances during a code review pays off in the long run, and increases the robustness of your code base. Another thing to look for, should this code be placed in your code base or extracted out into a library? As teams grow larger and different parts of your code base are developed at different times, and by different people, it will become more and more critical to make sure your code is organized, in a way your team can understand. If the code you're reviewing would make more sense in one of your utilities libraries, say so. Catching these kinds of issues should be encouraged and expected during your code reviews. If you can't answer these questions confidently, then you've probably found a valid design flaw, one that's justified to bring up in a code review. As a critical category of issues to look out for, try to always keep these questions in mind when you review someone else's code.

Contents