Code Review: From Toxic Nitpicking to Senior Engineering

Code Review is not a grammar school test. If your team spends time arguing about trailing commas or whether a variable should be named data or payload, you aren’t doing engineering—you’re practicing bureaucracy.

Real Code Review is the final line of defense against Overengineering and Logic Rot. Its goal isn’t to make the code ‘pretty’; its goal is to ensure the project doesn’t become unmaintainable trash in six months

1. The Hierarchy of Review (Stop Wasting Time)

Before you even open a Pull Request (PR), you must have a “Filter System.” If a human is checking things a machine can check, you are burning money.

• Layer 0: The Machines. Prettier, ESLint, Husky. If the brackets are wrong, the CI/CD should kill the build. Never mention syntax in a comment.

• Layer 1: The Logic. Does the code actually solve the ticket? Or did the dev get distracted and refactor the entire auth system instead of fixing a button?

• Layer 2: The Architecture. This is where the Senior shines. Is this a simple fix, or is the dev introducing a “God Object” that will haunt us forever?

2. Anti-Pattern: The Overengineering Trap

The most common “nuisance” in PRs is when a developer tries to show off. They use five design patterns where a simple if statement would suffice.

The Bad (Overengineered)

A developer is asked to calculate a discount. Instead of a function, they bring in a “Strategy Pattern” with three interfaces.

// DON'T DO THIS: The "I just learned Design Patterns" approach
interface DiscountStrategy {
    calculate(price: number): number;
}

class ChristmasDiscount implements DiscountStrategy {
    calculate(price: number) { return price * 0.9; }
}

class DiscountFactory {
    static getStrategy(type: string): DiscountStrategy {
        if (type === 'holiday') return new ChristmasDiscount();
        return { calculate: (p) => p };
    }
}

// All this just to multiply a number by 0.9
const finalPrice = DiscountFactory.getStrategy('holiday').calculate(100);

The Fixed (Pragmatic)

As a reviewer, your job is to strip this down. Unless the project has 50 different discount types that change daily, this is garbage.

// FIXED: Just write the logic.
const calculateDiscount = (price, type) => {
    const rates = { holiday: 0.9, clearance: 0.5 };
    return price * (rates[type] || 1);
};

const finalPrice = calculateDiscount(100, 'holiday');

Reviewer Note: “We don’t need a Factory for a single math operation. Keep it flat until we actually have 5+ strategies.”

3. The “Boolean Soup” vs. State Machines

During review, look for developers who use 5 different booleans to track one status. This is where bugs live.

The Bad (The Boolean Mess)

// Hard to track. What if isLoading and isError are both true?
const [isLoading, setIsLoading] = useState(false);
const [isError, setIsError] = useState(false);
const [isSuccess, setIsSuccess] = useState(false);

if (isLoading && !isError) { /* ... */ }

The Fixed (The State Machine)

Force the developer to use a single “Status” string or a State Machine. This makes the code impossible to break.

// FIXED: One source of truth.
const [status, setStatus] = useState('idle'); // 'loading', 'error', 'success'

if (status === 'loading') { /* ... */ }

4. Psychology: How to Comment Without Being a Jerk

If you write “This is stupid,” the dev gets defensive. If they get defensive, they stop learning. Your comments should be Objective Facts, not Subjective Opinions.

5. Security & Edge Cases (The “Dark” Review)

A Junior looks at the “Happy Path” (how it works when everything goes right). A Senior looks at the “Dark Path” (how it breaks).

What to look for in every PR:

1. Input Validation: Did they trust the user? (Spoiler: Never trust the user).

2. Null Checks: What if the API returns an empty array or null instead of an object?

3. Resource Leaks: Did they open a database connection or an event listener and forget to close it?

   Worth Reading

   Memory Management Overhead

   Understanding Memory Management Overhead in Python, Go, Rust, and Mojo Your Python worker hits 4GB RSS on a payload that should need 400MB. Your Go service P99 jumps from 8ms to 47ms every 90 seconds...

Example: The Silent Crash

// BAD: Assuming the user object always exists
const userEmail = response.data.user.email; 

// FIXED: Defend against the "undefined" crash
const userEmail = response.data?.user?.email || 'guest@krun.pro';

6. The “Bus Factor” Strategy

Code Review is secretly a knowledge-sharing tool. If only one person knows how the “Billing Module” works, and they get hit by a bus (or quit), the company is screwed.

• Rotation: Don’t let the same people review the same modules.

• Documentation: If a reviewer asks “Why is this done this way?”, and the answer is complex, that answer belongs in a code comment, not just in the PR chat.

7. The 15-Minute Rule

If a PR is more than 400 lines of code, the human brain stops finding bugs. It just starts “rubber-stamping” (clicking Approve to get it over with).

The Krun.pro Rule:

If the PR is a monster, break it down. Reviewers should spend no more than 15-20 minutes on a single session. If you are tired, you are useless.

Final Conclusion: The Goal is Ship, Not Perfection

The “Perfect Code” is the code that never gets shipped.

Don’t be the guy who blocks a release because of a missing semicolon. Be the guy who blocks a release because the database will melt under load.

The Checklist for Krun.pro Readers:

1. Does it solve the business problem?

2. Is it simple enough for a Junior to read?

3. Will it break if the input is weird?

4. Is it a “Clever” solution (bad) or a “Clear” solution (good)?