This is the fifth article in a series where we cover the book Software Engineering at Google by Titus Winters, Tom Manshreck, and Hyrum Wright. 📕 We will go over various aspects of software engineering as a process, including the importance of communication, iteration and continuous learning, well-thought-out documentation, robust testing, and many more.
Today we touch on two aspects of software engineering directly related to code - these are code style and code review. Let's dive in!
Don't make me think
Engineers love to think. Our job is almost all about thinking. And no wonder, figuring out solutions to problems requires deep thinking. However, it's sometimes hard to turn it off when we no longer have to do it. This explains why sometimes we come up with new ways to solve problems that already have established solutions - we reinvent the wheel. 😬
This is definitely true when it comes to code style and formatting. Code style rules help us manage complexity and build a maintainable codebase. A shared set of rules frames the engineering process so that it can scale up and keep growing.
Rules and guidance should aim to support resilience to time and scaling. For code formatting rules, it doesn't matter as much what the rule is but rather that it exists and is automatically enforceable. That's the point. There are plenty of resources and tools that help us automate code style, both formatting and alerting. We should figure out the rules that best suit our team and the project, integrate them, and let them do their job.
For any code rules, consistency is key. It makes code easier to read and understand for any engineer, regardless of their seniority. Relying on automation should rule out any comments around code style and formatting during the code reviews, letting us focus on what matters.
Speaking of code review. 👇
Code review is a must
Code review is one of the most fundamental processes of software engineering. Most teams follow this practice, as there are plenty of benefits that it brings. The book outlines the following benefits of code review.
Checks code correctness. Right after testing changes locally, review allows to check if the code is correct. It's a good practice to review your change before passing it on to others, which gives you a chance to spot mistakes early. Then, reviewers have an opportunity to do the same. Finally, creating a merge request should trigger a pipeline run, and automatically check the formatting, code style, build correctness, and test coverage.
Ensures the code change is comprehensible to other engineers. A code review is often the first test of whether a given change is understandable to a broader audience. This perspective is vitally important because code will be read many more times than it is written, and understanding and comprehension are critically important.
Enforces consistency across the codebase. Reviews give other engineers a chance to point out inconsistencies between the change and other parts of the codebase they are more familiar with. Code that is consistent and simple is easier to understand and easier for tools to update when it comes time for refactoring, making it more resilient. If a particular pattern is always done in one fashion in the codebase, it's easier to write a tool to refactor it.
Promotes team ownership. Code review has important cultural benefits: it reinforces to software engineers that code is not "theirs" but in fact part of a collective enterprise. When a change is reviewed and approved, the author of the change is no longer the only owner of that particular contribution. The responsibility is spread across other team members.
Enables knowledge sharing. Reviewing other people's code gives you insights into diverse coding styles, design patterns, and implementation strategies. You can learn from one another, develop a shared understanding of the codebase, and take a collaborative approach to problem-solving.
Despite the benefits, code review is inherently hard to do. If you think about it, this shouldn't come as a surprise. Code review is essentially having disagreements in a controlled environment. Very few people like that, so it's extremely important to have that in mind when writing comments. 💬
The answer to that issue in short is to be polite and professional. Remember that the code review is about the code, not about the person. Write actionable comments and provide necessary context or guidance. Conventional Comments is a great resource that helps write effective and respectful code review comments.
The final element of the review process is approving the code. The book provides a good rule of thumb for that.
Approve changes that improve the codebase rather than wait for consensus on a more "perfect" solution.
This focus tends to speed up code reviews. Other engineers can benefit from the improvements already available in the codebase, while you keep iterating on your feature. 🎉
Code review done right
So far we focused mostly on the reviewer side, but how successful code review is as a process depends to the same degree on the author of the change. Here are some of the best practices that the book highlights for preparing your change for review.
Write small changes. There's nothing worse for a reviewer than being presented with dozens of files with significant changes. A code review should ideally be easy to digest and focus on a single issue, both for the reviewer and the author. It allows you to narrow down what matters, reduces the downtime of waiting for review, and helps determine the source of a bug within changes later.
Write good change descriptions. The situation is even worse if you have no idea what you're about to review. It's likely that engineers won't be familiar with this area or feature. Besides having a good title, write a description that gives others a wider context behind why this change is needed and how it works, outlining any implementation details - basically anything that would help when reviewing the change.
Keep reviewers to a minimum. You want to get other peoples' perspectives on your change. But you also want the process to be efficient and focused, so keep in mind that the cost of additional reviewers quickly outweighs their value. With fewer reviewers, communication is streamlined, there's a greater sense of accountability, and you minimize the time developers wait for feedback.
Automate where possible. Given how frequently engineers review code, we should automate as much as possible. The readily available tools allow us to cover a lot of areas, such as static analysis, code style and formatting, checking the build correctness, and running tests to check if they pass. All of that should be a part of an automated pipeline so that engineers can focus on reviewing the logic rather than checking how the code looks.
Having all of that in place, the book also recommends running through a checklist before submitting a change and requesting reviews. Checklists work great because they are actionable and can be automated.
Is this change necessary? It might sound obvious, but make sure that all of the code in your change is used and you're not overpolishing something that already works. It helps prevent code that's not needed (YAGNI Principle) and rule out hasty abstractions (AHA Principle).
Does it improve the codebase? It's a good thing to keep in mind, that any change that you introduce should bring in more benefits than costs to the codebase. That way, with each change the overall health of the codebase increases.
Is it tested? Adding tests that give you confidence that the modification works as expected should be a part of (almost) every change. Beyond checking for correctness, it can also partially serve as documentation and give reviewers more context of what they can expect.
Is the scope of the change limited? This ties back to writing small changes. Make sure that your change is digestible. It's fine to change a lot of files at once when you're running automated or repetitive changes. However, the more complex your change is, the fewer files your merge request should probably touch.
Conclusion
That's it for today. Combining the right code style rules and an effective code review process allows us to build a maintainable codebase. Here's a short summary of things we went through:
code style rules help us manage complexity and build a maintainable codebase
it doesn't matter what the rule is but rather that it exists and is automatically enforceable
code review allows to ensure code correctness, readability, and consistency
reviewing code builds shared ownership and enables knowledge-sharing
as code owner, write small changes with good descriptions, keep the reviewers to the minimum, and automate where possible
before submitting a change, make sure it is necessary, it is limited in scope, it improves the codebase and it is tested
Next, we will discuss something that goes hand in hand with your code - documentation. See you! 👋
If you liked the article or you have a question, feel free to reach out to me on Twitter‚ or add a comment below!
Further reading and references
