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 arent doing engineering—youre practicing bureaucracy.

Real Code Review is the final line of defense against Overengineering and Logic Rot. Its goal isnt to make the code pretty; its goal is to ensure the project doesnt 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 dont 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?

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: Dont 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.

Dont 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)?