Why Code Review Matters: Mastering Design Principles for Cleaner Code

Preface

As a member of the company’s Go code‑review committee, I have examined many reviews and noticed that many engineers need to improve both their review skills and code quality. I want to share some ideas and approaches.

Why should technical staff, including leaders, do code review?

"Talk is cheap, show me the code." Knowing concepts is easy; applying them is hard. Many people only recognize a design principle without actually practicing it, leading to a false sense of security.

Code is the concrete manifestation of design ideas. During review, we can turn abstract discussions into concrete communication, learn from each other, and converge on the best practices for the team. Even leaders who do not write code must still provide solid guidance on best practices.

Why should reviewers think about and summarize best practices?

An architect masters many design ideas, principles, and toolchains, and can control the maintainability, testability, and operational quality of a 300k‑line project.

Technical tricks ("奇技淫巧") – many clever tricks exist, but they often have limited engineering value.

Foundational domain knowledge – e.g., John Carmack’s graphics breakthroughs.

Theoretical research – e.g., early large‑vocabulary speech recognition.

Product success – practical outcomes.

Best practices – achievable by anyone who follows the architect’s definition.

Improving code quality is essentially a continuous process of refining best‑practice methodologies and implementation details.

Root Causes of Bad Code

Duplicate code

// BatchGetQQTinyWithAdmin 获取QQ uin的tinyID, 需要主uin的tiny和登录态
// friendUins 可以是空列表, 只要admin uin的tiny
func BatchGetQQTinyWithAdmin(ctx context.Context, adminUin uint64, friendUin []uint64) (
    adminTiny uint64, sig []byte, frdTiny map[uint64]uint64, err error) {
    var friendAccountList []*basedef.AccountInfo
    for _, v := range friendUin {
        friendAccountList = append(friendAccountList, &basedef.AccountInfo{
            AccountType: proto.String(def.StrQQU),
            Userid:      proto.String(fmt.Sprint(v)),
        })
    }
    // ... build request body ...
    return nil
}

When the initial protocol is poorly designed, each new developer rewrites similar request‑building code, leading to duplicated fragments that are hard to refactor and cause bugs.

Early optimization

Over‑optimizing a function that initially looks fine can quickly make it unreadable and hard to maintain.

Over‑reliance on OOP and inheritance

Excessive inheritance trees make it difficult for junior engineers to understand and modify code. Prefer composition over inheritance to keep each component understandable and replaceable.

No design at all

Starting a project without any design leads to massive, monolithic files that are impossible for others to maintain.

Metaphysical Thinking

Engineers should move from merely following tutorials to building a personal framework of principles that connect technical details with system requirements.

Model Design

Without reading the OAuth2.0 RFC, developers often reinvent a broken version of the protocol. Proper model design helps accommodate evolving product requirements without constant rewrites.

UNIX Design Philosophy

"People who don’t understand Unix will inevitably reinvent a broken version of it." The article highlights timeless engineering culture, such as the KISS principle, composition, and transparent design.

Keep It Simple Stupid!

True simplicity means fully understanding the problem before crafting a concise solution, not just picking the first obvious answer.

Composition Principle

Use composition to build flexible modules; inheritance can become a maintenance nightmare in large, evolving systems.

Frugality Principle

Write the smallest program that works; avoid unnecessary code bloat that hampers future changes.

Transparency Principle

Code should be easy to read and reason about; variable and function names must convey intent clearly.

Verbosity Principle

Avoid overly clever naming that obscures meaning; simple, conventional names reduce collaboration cost.

Silence Principle

Log only what is necessary; excessive logging drowns out real errors.

Remedy Principle

When an exception occurs, abort immediately and provide detailed error information.

Practical Tips (Go‑specific)

Enforce 100% code‑format compliance.

Files must not exceed 800 lines; split when they do.

Functions should stay under 80 lines; refactor otherwise.

Nesting depth should not exceed four levels; use early returns.

if !needContinue {
    doA()
    return
}
// else branch omitted for clarity

Organize code hierarchically: package → directory → file → struct → function. Avoid redundancy in definitions.

Use multi‑level directories even if some levels contain a single subdirectory to keep logical grouping clear.

When code violates design principles, refactor locally immediately to prevent quality decay.

Prefer composition over inheritance for flexible, maintainable systems.

Log sparingly but include essential context; avoid noisy logs.

Ask questions early; don’t fear “low‑level” doubts.

Adopt trpc and align with leadership on code‑quality goals.

Eliminate duplication relentlessly.

Main‑line Development

Keep review size under 500 lines to ensure thorough inspection and reduce the burden of large‑scale changes.

Continuous integration (CI) resources are abundant; study them.

Recommended Reading

Read "The Art of Unix Programming" for timeless engineering wisdom, and consider "Software Engineering at Google" for modern practices.

Author

Author: cheaterlin, Tencent PCG backend engineer