Effective Code Review: Enhancing Code Quality and Programming Skills for New Developers – ITFROMZERO

Table of Contents

Quick Start: Code Review in 5 minutes

You’ve received a Pull Request (PR) from a colleague, and they want you to review the code before merging it into the main branch. Don’t worry if you’ve never done this before! I’ll guide you on how to get started immediately:

Open the PR on the platform (GitHub, GitLab, Bitbucket): You’ll usually receive a link. Click it.
Read the PR Description: See what your colleague wants to change and why. This summary helps you understand the context of the changes.
Check the changes (Files changed): Go through each file one by one. Focus on new lines of code, and parts that have been modified or deleted. Tools like GitHub often highlight these changes with green (additions) or red (deletions).

Leave a Comment: If you find anything unclear, improvable, or erroneous, leave a comment directly on that line of code. Try to be specific about the issue you’ve found.

# Example comment on GitHub/GitLab
# @username: I find this if condition a bit complex. Could you simplify it or break it down into a separate function for better readability?
# Or: The logic for handling the null status case seems unhandled. Could null checks be added here?

Submit Feedback (Submit review): After reviewing, you can choose “Approve,” “Request changes,” or “Comment” (to leave general comments only). If there’s a serious issue, select “Request changes.”

You’ve now completed a basic code review! Let’s dive deeper.

What is Code Review and Why is it Important?

Code review, or “source code inspection,” is the process where one or more other developers examine your source code to find errors, improve quality, and ensure it meets project standards. It’s not a “nitpicking” session but a collaborative activity to jointly create a better product, where everyone learns from each other.

Why do I say code review is so important?

Enhance code quality: Two pairs of eyes are better than one. Reviewers can detect logical errors, potential bugs, or scenarios you might have overlooked. This helps minimize errors when code is deployed to production, preventing unnecessary incidents.
Share knowledge and experience: When you review someone else’s code, you learn how they solve problems and the design patterns they use. Conversely, others reviewing your code will also gain a better understanding of the software, thereby strengthening the team and reducing reliance on any single individual.
Ensure standard compliance: Every project and every team has a set of coding rules (coding style, naming conventions). Code review helps ensure everyone adheres to them, keeping the source code consistent, readable, and maintainable.
Early error detection and correction: Fixing errors during the review stage is much easier and cheaper than discovering them after the product has been deployed, which can impact users and require significant time and effort to resolve.
Improve programming skills: This is one of the fastest ways to develop yourself. When you know your code will be read by others, you will automatically write more carefully, clearly, and structured. When you review someone else’s code, you must think critically, learn from their solutions, and thereby expand your knowledge and experience.

I recall that in my most recent web app project with 5 developers, my team and I implemented a stricter code review process. As a result, we significantly reduced the number of bugs needing fixes after deployment, and the pace of completing new features also increased because we spent less time revisiting old errors. I observed a clear boost in the entire team’s productivity, and everyone felt more comfortable with their code receiving feedback for improvement, rather than being seen as personal mistakes.

Common Types of Code Review

Over-the-shoulder review: You and a colleague sit together and review the code. This method is fast and direct but makes it difficult to track feedback and is not suitable for remote teams.
Email pass-around: Code is sent via email. This method is quite old-fashioned and ineffective for large changes or when multiple reviewers are needed.
Tool-assisted review (Pull Request/Merge Request): This is the most common method today. Using source code management tools like GitHub, GitLab, Bitbucket, all changes are encapsulated within a “Pull Request” (PR) or “Merge Request” (MR), making discussions and tracking very convenient and providing clear history.

Effective Code Review Process: From A to Z

For a code review process to be truly effective, both the PR creator (author) and the reviewer need to be responsible and follow the correct steps.

1. Pull Request Creator (PR Author): Thorough Preparation

Before submitting code for others to review, make sure you’ve done these things well:

Write a clear PR description: This is the “summary” for the reviewer. I always try to write it in detail:
- What is the purpose of this change? (Fix bug, add new feature, refactor?)
- What problem does it solve?
- What are the main changes in the code?
- Are there specific areas that need more review?
- Does the reviewer need to run the code for testing? If so, provide instructions.
Self-review the code before submitting: Review your own code first. You can find and fix many minor errors yourself before others see them. This not only saves time for both parties but also helps you improve your self-assessment skills.
Run unit/integration tests: Ensure that all tests have passed. If you add new features or fix bugs, write additional tests for them. Code without tests is code with many potential risks.

Example: Effective PR Description

# PR Title: Feat: Add functionality to display product list by Category

**Purpose:**
Add API endpoint `/api/products?category_id={id}` to retrieve product list by category ID.
The frontend will use this API to display products when a user selects a specific category.

**Problem Solved:**
Previously, there was no way to filter products by category, making it difficult to manage product display by type.

**Key Changes:**
- `src/controllers/productController.js`: Added `getProductsByCategory` function to handle requests.
- `src/routes/productRoutes.js`: Updated `GET /api/products` route to accept `category_id` query parameter.
- `src/services/productService.js`: Added database query logic to filter products based on `category_id`.

**Note for reviewer:**
- Please thoroughly check the filtering logic in `productService.js`, especially when `category_id` is invalid or not passed.
- Unit tests for `getProductsByCategory` have been written in `tests/unit/productService.test.js` to ensure correctness.

**Test Instructions:**
1. Clone the latest repository.
2. Run `npm install` and `npm start` to start the application.
3. Open Postman or use `curl` to test the API:
   - Get products by category ID 1: `curl -X GET "http://localhost:3000/api/products?category_id=1"`
   - Get all products (when no category_id is provided): `curl -X GET "http://localhost:3000/api/products"`

Example: Running tests with Python/Node.js

To ensure your code doesn’t break anything existing, run the tests:

# For <a href="https://itfromzero.com/en/development-vi-en/python-vi-en-development-vi-en/processing-csv-and-excel-files-with-python-pandas-comparing-approaches-and-practical-implementation-guide.html">Python projects</a> using pytest
pytest tests/

# For JavaScript/TypeScript projects using Jest or similar
npm test
# or
yarn test

2. The Reviewer: Focus and Constructive Feedback

When you are the reviewer, remember that the goal is to make the code better, not to criticize personally. All feedback should aim to improve quality.

Understand the context of the change: Read the PR description thoroughly and take time to understand the purpose and solution proposed by the PR creator. If necessary, don’t hesitate to ask directly for clarification.
Check critical aspects:
- Logic: Does the code work as expected? Does it handle all edge cases? (e.g., empty data, negative values, connection errors…)
- Complexity: Is the code too complex or difficult to understand? Can it be simplified by breaking down functions or using more optimal algorithms?
- Performance: Does this change negatively impact system performance? (e.g., too many nested loops, inefficient database queries).
- Security: Are there any new security vulnerabilities? (e.g., SQL Injection, XSS, exposure of sensitive information).
- Readability & Maintainability: Are variable and function names clear and easy to understand? Are there comments in complex areas? Does the code adhere to the team’s coding style? Readable code will be easier to maintain and extend later.
- Tests: Do the new tests cover enough cases? Do existing tests still pass after the changes?
Provide specific and constructive feedback:
- Always explain why you are suggesting a change. Pointing out a problem without a reason will be hard to persuade.
- Propose solutions instead of just saying “this is wrong.” Think about how you would do it if you were the one writing the code.
- Use a friendly, encouraging tone. Avoid critical language.
- Categorize feedback: “Blocking” (must be fixed before merging, usually serious errors, bugs) and “Suggestion” (can be improved but not mandatory, usually related to style or minor performance).

Example: Effective review comment

Instead of just saying “This code is bad” or “This is wrong,” be specific and make suggestions:

// Before:
- def get_active_users():
-     users = database.query("SELECT * FROM users WHERE status = 'active'")
-     return users

// After (reviewer's comment):
+ def get_active_users():
+     """
+     Returns a list of all users with 'active' status.
+ 
+     Feedback (Blocking): I see you're directly concatenating SQL strings. This can lead to SQL Injection vulnerabilities if input data is not carefully sanitized.
+     I suggest using an ORM (like SQLAlchemy for Python) or prepared statements for safer database queries.
+ 
+     Feedback (Suggestion): Additionally, if the user list can be large, I think it's worth considering adding pagination to improve performance when loading data.
+     """
+     # Example using a prepared statement (assumed usage)
+     # users = database.execute("SELECT * FROM users WHERE status = ?", ('active',))
+     # Example using an ORM (like Django ORM or SQLAlchemy)
+     # users = User.objects.filter(status='active').all()
+     users = database.query("SELECT * FROM users WHERE status = 'active'")
+     return users

This comment not only points out the issues (SQL Injection, lack of pagination) but also proposes specific solutions (ORM/prepared statements) and explains the benefits (security, performance), while clearly categorizing the feedback.

3. Discussion and Refinement

Two-way communication: The PR creator doesn’t necessarily have to accept every piece of feedback. If there’s a valid reason to keep the code as is, explain it clearly. The goal is to find the best solution for the code, not to determine who is right or wrong.
PR creator updates code: Based on the feedback, the PR creator modifies and updates the code. Then, they will mark comments as resolved or request the reviewer to re-examine the new changes.
Reviewer approves or requests further changes: Once all issues have been satisfactorily resolved, the reviewer will approve the PR. If there are still points that need fixing, they will continue to request changes until the code reaches the desired quality.

Enhance Your Code Review Skills

Code review is not just about looking at other people’s code; it’s an excellent opportunity for the entire team to grow together and for you to upgrade your own skills.

Utilize Supporting Tools

Don’t try to do everything manually. These tools will help you a lot, automating repetitive tasks:

Linters and Formatters:
- Linters (e.g., ESLint for JavaScript, Pylint for Python) help detect syntax errors, code style issues, and potential bugs. They are like a strict teacher helping you clean up your code.
- Formatters (e.g., Prettier for JavaScript, Black for Python) automatically format code according to a set of rules, keeping the code consistent and beautiful, so you don’t have to worry about spaces or line breaks.
I always encourage the team to run linters/formatters automatically before committing code. This solves 80% of style issues before review, allowing us to focus on logic.

Example: Running linter/formatter
```
# Run Black to format Python code in the current directory
black .

# Run ESLint to check and automatically fix JavaScript/TypeScript style errors
eslint --fix src/
```
Static Analysis Tools: These tools (e.g., SonarQube, Bandit for Python) go deeper, searching for security vulnerabilities, excessive complexity, and potential architectural issues that linters cannot detect. They provide a more comprehensive view of code quality and risks.
Integrate into CI/CD (Continuous Integration/Continuous Delivery): Automatically run linters, formatters, and tests in the CI/CD pipeline. If there are errors, the PR will not be merged, forcing the developer to fix them first. This saves a lot of review time, as basic errors are automatically caught.

Focus on Value, not trivialities

When reviewing, prioritize major issues that have a high impact on the project and users:

Incorrect business logic.
Serious security vulnerabilities.
Performance issues significantly affecting user experience.
Major architectural changes that could hinder future scalability.
Code prone to system crashes under certain conditions.

Minor details like extraneous semicolons, suboptimal (but still understandable) variable naming, or an incorrect whitespace… can be pointed out as suggestions but should not be a reason to block a PR. Trust the automation tools mentioned above to handle these issues.

Learn from Every Review

When you review someone else’s code: You will encounter many different ways to solve problems. Think about why they did it that way, whether their solution is better than yours, and what you can learn from it.
When your code is reviewed: Don’t take it as personal criticism. See it as an opportunity to learn and improve. Remember good feedback to apply next time, helping you become a better developer every day.

Practical Tips for Friendly and Effective Code Review

Keep PRs small: A PR should contain only one change, addressing a specific issue. Reviewing a PR with 5000 lines of code changes is torture and makes it very easy to miss errors. The ideal target is PRs under 200-300 lines of code changed.
Don’t review for too long: The human brain has limits on its ability to concentrate. Try to review for about 30-60 minutes. If a PR is too large, suggest the creator break it down or review it in multiple sessions.
Create a positive atmosphere: Start the review with compliments if appropriate. “I like how you handled this authentication part, it’s very clean and easy to understand!” will help the PR creator feel more comfortable receiving other feedback.
Use emojis: Sometimes, an emoji can convey emotion better than words. For example:
- `👍` for a good piece of code or when you agree.
- `🤔` when you are thinking or don’t understand an issue.
- `😅` if you find a cute little error or something humorous.
Don’t be afraid to ask: If you don’t understand a part of the code or its purpose, ask immediately. No one expects you to know everything. Asking also helps the PR creator reflect on whether their code is clear and understandable enough for others.
Meet in person if necessary: For complex issues, or when too many arguments via comments lead nowhere, proactively suggest a short meeting or “over-the-shoulder review” to resolve it faster and avoid misunderstandings.

Conclusion

Code review is not just a technical process but also a crucial cultural aspect in modern software development. It’s a bridge that helps your team collectively enhance product quality and sustainably develop personal skills.

By practicing an effective code review process, you will not only write better code but also become a more capable developer, with critical thinking and strong collaboration skills. Start applying it today and view code review as an essential tool on your career development journey!