Respectfully Reviewing Your Coworker's Code Changes

Respectfully Reviewing Your Coworker's Code Changes


images/respectfully-reviewing-your-coworker-s-code-changes.webp

When it comes to code review, it’s not just about what you say, but also how you say it. The art of reviewing a coworker’s code change is as much about diplomacy as it is about technical acumen. The goal is to enhance the code’s quality while maintaining a positive and collaborative team environment.

Understanding the Context

Before diving into the code, it’s crucial to understand the context of the changes. What problem is this code solving? Is it a new feature, a bug fix, or a performance improvement? This context sets the stage for your review and ensures that your comments are relevant and constructive.

Starting with the Positive

Begin your review by acknowledging what’s been done well. This could be anything from clever solutions to complex problems to good coding practices. For instance, if you’re reviewing for a more junior engineer who doesn’t tend to write extensive unit tests, and you see that they’re making an effort to test a bit more thoroughly, you can comment on this like:

I’m loving all of these tests! Thank you for building confidence in these changes.

This approach sets a constructive tone and makes your colleague more receptive to forthcoming suggestions.

Providing Constructive Feedback

When it comes to pointing out areas for improvement, specificity is key. Vague comments like “this looks wrong” are not helpful. Instead, offer clear, actionable feedback. If you’re reviewing a Python script and spot a potential security vulnerability, for example, with SQL injection, provide a specific recommendation:

query = f"SELECT * FROM users WHERE username = '{username}'"
cursor.execute(query)

This is a potential SQL injection vulnerability. Consider using parameterized queries:

query = "SELECT * FROM users WHERE username = ?"
cursor.execute(query, (username,))

Linking to resources or documentation is also beneficial. For SQL injection, you might include a link to OWASP’s SQL Injection Prevention Cheat Sheet.

Focusing on the Code, Not the Coder

It’s imperative to remember that the focus of a review is the code, not the person who wrote it. Avoid personal language and focus on the code’s behavior and structure. Instead of saying “You didn’t handle exceptions here,” rephrase it to “Exceptions aren’t handled in this section.”

Encouraging Discussion and Alternative Approaches

Code review is not a one-way street. Encourage dialogue by asking questions and suggesting alternatives. This not only helps in clarifying certain decisions but also opens up avenues for learning and collaboration. For instance:

List<String> filteredList = new ArrayList<>();
for (String item : list) {
    if (item.contains("keyword")) {
        filteredList.add(item);
    }
}

Have you considered using a stream here for better readability? Here’s the alternative:

List<String> filteredList = list.stream()
                                .filter(item -> item.contains("keyword"))
                                .collect(Collectors.toList());

Balancing Nitpicks and Fundamental Issues

Distinguish between critical issues that impact the functionality, security, or performance of the code and minor “nitpicks” that are more about personal coding preferences. While it’s important to maintain a high code standard, focusing too much on trivial matters can be demoralizing and counterproductive.

Encouraging Best Practices

Use the code review process as an opportunity to reinforce best practices. If you notice recurring patterns that deviate from best practices, bring them up in the review. For example, if a colleague consistently forgets to sanitize user inputs in a web application, point it out with references to best practices for web security:

element.innerHTML = userContent;

Remember to sanitize user inputs to prevent XSS attacks. Consider using a library like DOMPurify:

const safeContent = DOMPurify.sanitize(userContent);
element.innerHTML = safeContent;

Being Timely and Efficient

Timeliness in code reviews is crucial. Delayed reviews can bottleneck the development process. Strive to conduct reviews promptly while being thorough. Also, be efficient with your feedback – if you see multiple instances of the same issue, mention it once and suggest a comprehensive review for similar issues.

Leveraging Automation Where Possible

Automated tools can greatly assist the code review process by catching common mistakes and enforcing coding standards. Tools like ESLint for JavaScript or Pylint for Python can automate the detection of stylistic issues, allowing reviewers to focus on more complex aspects of the code. Integrating these tools into your workflow can save time and standardize code quality.

Building a Positive Review Culture

Finally, the overarching goal should be to build a positive and constructive review culture. Encourage your team to view code reviews as opportunities for learning and improvement rather than criticism. Celebrate good coding practices and improvements in code quality over time.

In conclusion, respectful code reviewing is a skill that balances technical expertise with interpersonal sensitivity. By focusing on constructive feedback, encouraging discussion, and maintaining a positive attitude, you can contribute significantly to the quality of your team’s code and the overall health of your development environment. Remember, a well-conducted code review not only improves the code but also strengthens team dynamics and fosters a culture of continuous learning and improvement.


About PullRequest

HackerOne PullRequest is a platform for code review, built for teams of all sizes. We have a network of expert engineers enhanced by AI, to help you ship secure code, faster.

Learn more about PullRequest

PullRequest headshot
by PullRequest

February 2, 2024