Why Code Reviews Matter: Boost Quality, Reduce Debt, and Empower Teams

Reference Materials

"How Big Tech Companies Play Code Review" – Liang Songhua, JD Senior Engineer

"Learning How Facebook Leverages Code Review for Efficiency" – Ge Jun, Former Facebook Tech Lead

"Which Code Review Method Fits My Team" – Ge Jun, Former Facebook Tech Lead

"Talking About Code Review" – Xiong Yi, Oracle Chief Software Engineer

"6 Common Problems in Code Review" – Songhua Pi Dan, InfoQ

"Code Review: Hope and Sorrow" – Hu Feng, JD Chengdu Research Institute Technical Expert

Code, like entropy, tends to decay without external force. When code accumulates, quality degrades. This article offers a practical guide to ensure high‑quality code, especially when rewriting large sections.

Why Review?

Consequences of Not Reviewing

Poor code quality leads to unstable products, bugs, and damaged customer satisfaction.

It also raises future maintenance costs, especially when personnel changes occur.

Unstandardized, unreadable code causes friction during hand‑offs, leading to ad‑hoc fixes and further degradation over years.

Technical debt can cause anything from localized glitches to complete rewrites.

From physics, entropy shows that without external influence, systems become more chaotic; thus, standards and reviews are vital.

From software design, long‑term change requires stable, maintainable code.

In product development, lack of review cannot sustain long‑term success.

Benefits of Review

Code review is a low‑investment, high‑output activity that yields personal growth and team knowledge sharing.

Six major benefits:

Early bug and design issue detection – catching problems early reduces fix costs.

Improved engineering skills – peer feedback raises code quality.

Team knowledge sharing – reviewed code becomes shared knowledge and documentation.

Targeted quality improvements – experts can focus on security, performance, UI, etc.

Unified coding style – often automated with tools.

Social function – knowing code will be reviewed changes coding habits positively.

Review Challenges and Controversies

Most disputes focus on reasons or excuses for avoiding reviews.

1) Time‑consuming effort

Tight deadlines and overtime make developers reluctant to allocate time for reviews.

2) Team‑building concerns

Disagreements during reviews can strain relationships.

3) Subjective preferences

Reviewers may impose personal style preferences.

4) Unbalanced team composition

Too many inexperienced members and too few senior reviewers reduce effectiveness.

5) Resistance to review

Some confident developers view review as nit‑picking.

6) Formalism and poor execution

When done poorly, reviews become a checkbox exercise.

Understanding Review Forms

Two main categories: manual review and automated static analysis. Manual review can be pure online or offline (screen‑sharing) – the latter is recommended for deeper discussion.

Review Objects

Any substantial code change should be reviewed; minor tweaks may need only a quick peer check.

Participants

All stakeholders closely related to the business flow should attend, such as the developer’s mentor and the next‑stage owner.

Developer’s mentor

Colleague responsible for the subsequent workflow step

Involving them prevents information asymmetry and missed changes.

What to Focus On

Avoid subjective style enforcement; instead, document standards.

Goal: maximize team benefit.

Key focus points:

Obvious logical errors

Compliance with coding standards

Readability and maintainability

Adherence to design patterns

Do not over‑focus on:

Whether code runs

Business requirement coverage

Scenario coverage

These aspects are jointly ensured by developers and testers.

Review Process

Six steps to keep work uninterrupted:

Agree on a specification document to avoid subjective bias.

Schedule reviews (e.g., every Monday) based on change size and release timeline.

Provide supplemental material: background, code scope, impact.

Conduct the review; decide if a second round is needed based on severity.

Enforce essential elements (monitoring, comments, etc.); reject if missing.

Iteratively improve coding standards based on review outcomes.

Key Operations

Increase commit atomicity – split large features into small, daily commits.

Improve commit messages – include title, description, and test status.

Example of a good commit message structure:

Title – concise (≤70 characters).

Detailed description – purpose, approach, implementation summary.

Test situation – what was tested, results, performance/security checks.

Related info – task ID, sprint link, etc.

Key Principles

Mutual respect – reviewers spend time reading unfamiliar code; authors should make review easy.

Discussion‑oriented – reviews aim to discuss, not judge; avoid preaching.

Common Review Issues with Code Samples

Missing comments and change notes

public Article GetArticleById(long id) {
    return null;
}
public MaterialPO GetMaterialById(long id) {
    return null;
}
public ArticleVO GetArticleByIdWithoutStatus(long id) {
    return null;
}

Improper parameter handling

// Bad example
Boolean SaveError(List<ShareDetail> list) {
    // only checks for null, not inner nulls
    if (list.IsEmpty()) {
        return false;
    }
    int id = list.get(0).getId(); // assumes all ids are same
    // save data
    return Save(id, list);
}

// Good example
Boolean SaveRight(int id, List<ShareDetail> list) {
    if (id == null || CollectionUtil.hasNull(list)) {
        return false;
    }
    return Save(id, list);
}

Excessive variable scope

public static void main(String[] args) {
    ShareDetail shareDetail1 = new ShareDetail();
    varErrorStep1(shareDetail1);
    ShareDetail shareDetail2 = varRight();
}

public static void varErrorStep1(ShareDetail shareDetail) {
    shareDetail.setId(111);
    varErrorStep2(shareDetail);
}

public static void varErrorStep2(ShareDetail shareDetail) {
    shareDetail.setName("hello");
}

public static ShareDetail varRight() {
    ShareDetail shareDetail = new ShareDetail();
    shareDetail.setId(111);
    shareDetail.setName("hello");
    return shareDetail;
}

Lack of staged results

// Bad example
public void SetpError() {
    Boolean flag = false;
    List<Integer> list = new ArrayList<>();
    for (Integer detail : list) {
        if (detail > 0) {
            flag = true;
        }
    }
    if (flag) {
        return;
    }
}

// Good example
public void SetpRight() {
    List<Integer> list = new ArrayList<>();
    for (Integer detail : list) {
        if (detail > 0) {
            return; // early exit eliminates flag
        }
    }
    // further steps...
}

Logging issues

// Bad example
public Boolean logError(Integer id, List<ShareDetail> list) {
    if (id == null || CollectionUtil.hasNull(list)) {
        return false;
    }
    logger.info("logError run");
    try {
        return shareDao.save(id, list);
    } catch (Exception e) {
        logger.error("save error");
        logger.error("save error e:{0}", e.getMessage());
    }
    return false;
}

// Good example
public Boolean logError(Integer id, List<ShareDetail> list) {
    if (id == null || CollectionUtil.hasNull(list)) {
        return false;
    }
    logger.info("logError run");
    try {
        return shareDao.save(id, list);
    } catch (Exception e) {
        logger.error("save error");
        logger.error("save error id:{}, e:{}, list:{}", id, e, list == null ? list : JSONObject.toJSON(list), e);
    }
    return false;
}

Conclusion

To make teams embrace code review, its quality must be high; otherwise reviews become formalities. Clear definition of review form, objects, participants, focus, process, and common pitfalls is essential.

When time permits, involve testers so they understand code changes and can better define test boundaries.