Mastering Code Review: Proven Practices to Boost Code Quality and Team Collaboration

Why Code Review?

Early defect detection – catches logic errors, performance risks, and misunderstandings before they reach production.

Improves code quality – enhances robustness, design soundness, and elegance.

Unifies standards and style – promotes readable, maintainable code for future developers.

Prevents architectural decay – involves the whole team in preserving architecture.

Knowledge sharing – each review spreads expertise and helps newcomers learn business details.

Team consensus – repeated discussions build shared understanding of design principles.

Lessons from Other Companies

Company A

Require at least two reviewers before a change can be merged.

Guidelines: accept changes that improve overall quality even if imperfect; balance effort and impact.

Principles: base decisions on objective technical data, follow style guides, respect design principles, avoid degrading code quality.

Review focus: design, functionality, complexity, tests, naming, comments, style, documentation.

Company B

Dedicated review phase: e.g., 2 days of a 5‑day sprint for self‑review, cross‑review, and集中 review.

Metrics: reviews per KLOC, non‑trivial issues per KLOC.

Company C

Introduce a Code Owner role; every change must be approved by the owner.

Make review participation transparent; anyone can review any change.

Tie review performance to individual evaluation.

Our Code Review Process

As a Submitter

Timing : start the review early; keep changes small.

Size : aim for < 400 lines; larger changes reduce defect detection and review speed.

Clear intent : provide a descriptive title (required) and a detailed description (optional).

Tooling : use IDE plugins (e.g., Alibaba’s p3c) for real‑time rule enforcement.

As a Reviewer

Scope

Logic : functional completeness, design consistency, security risks, performance hazards, test coverage.

Quality : coding standards, readability, simplicity (KISS/DRY), maintainability, extensibility, testability.

Tips

Finish reviews promptly.

Avoid chasing perfection; focus on actionable feedback.

Make sure comments are resolved.

Use constructive language, not rhetorical questions.

Avoiding Formality

Gain reviewer buy‑in by showing tangible benefits.

Keep sessions informal and collaborative.

Focus on problem solving, not personal blame.

Common Code Issues Discovered in Reviews

DRY (Don’t Repeat Yourself)

Repeated code should be extracted into shared methods or classes. Over‑abstracting can violate cohesion; follow the “Rule of Three”.

private static final String MONEY_DISPLAY_TEXT_PATTERN = "%s %s";

private BillVO convertBillDTO2BillVO(BillDTO billDTO) {
    if (billDTO == null) return null;
    BillVO billVO = new BillVO();
    billVO.setCostDisplayText(buildMoneyDisplayText(billDTO.getCost()));
    billVO.setSaleDisplayText(buildMoneyDisplayText(billDTO.getSale()));
    billVO.setGrossProfitDisplayText(buildMoneyDisplayText(billDTO.getGrossProfit()));
    return billVO;
}

private String buildMoneyDisplayText(Money money) {
    if (money == null || money.getAmount() == null) return StringUtils.EMPTY;
    return String.format(MONEY_DISPLAY_TEXT_PATTERN, money.getCurrency(), money.getAmount().toPlainString());
}

Primitive Obsession

Replace raw primitives with small domain objects (e.g., Money, Range) to improve semantics and reuse.

Distributed Lock Issues

Always handle lock acquisition failure and ensure lock release; encapsulate lock logic in a utility.

private void process(String orderId) {
    try {
        boolean lockSuccess = lockService.tryLock(LockBizType.ORDER, orderId);
        if (!lockSuccess) {
            // retry or throw
            return;
        }
        // business logic
    } finally {
        lockService.unlock(LockBizType.ORDER, orderId);
    }
}

Encapsulate with a helper method:

private Boolean doProcess(String orderId) {
    // business logic
    return Boolean.TRUE;
}

public <T> T executeWithLock(LockBizType biz, String id, Supplier<T> work) {
    if (!lockService.tryLock(biz, id)) {
        throw new LockException("try lock fail");
    }
    try {
        return work.get();
    } finally {
        lockService.unlock(biz, id);
    }
}

Pagination Problems

Never return an unpaginated list for large tables; enforce reasonable page sizes and validate parameters.

private Page<OrderDTO> queryOrderList(OrderPageQuery query) {
    Preconditions.checkNotNull(query, "查询条件不能为空");
    Preconditions.checkArgument(query.getPageSize() <= MAX_PAGE_SIZE, "分页size不能大于" + MAX_PAGE_SIZE);
    long total = orderMapper.count(query);
    if (total == 0) return PageQueryUtil.buildPageData(query, null, 0);
    List<OrderDO> list = orderMapper.list(query);
    List<OrderDTO> dtoList = orderConverter.doList2dtoList(list);
    return PageQueryUtil.buildPageData(query, dtoList, total);
}

Conclusion

Effective code review sharpens code quality, spreads knowledge, and safeguards architecture; by following the practices above, teams can continuously improve their software craftsmanship.