Code Critique

Systematic methodology for identifying code problems and communicating them with precision, specificity, and quantifiable evidence. No hedging, no softening, no vague suggestions.

Quick Start

Full code critique:

Perform a brutally honest code critique of this file. Identify all issues with specific line numbers, categories, and severity scores.

Targeted review:

Review this function for violations. Name each code smell, cite the line, and score severity 1-10.

Pre-PR audit:

Audit this code before PR submission. List every issue that would fail review, with exact locations.

Core Principles

The Three Commandments

1. Be Direct - State the problem, not a suggestion
2. Be Specific - Line numbers, counts, exact violations
3. Be Quantitative - Numbers, metrics, comparisons

Language Standards

Never Say	Always Say
"Perhaps consider..."	"This violates X because Y"
"It might be nice if..."	"Line 47 requires Z"
"Generally speaking..."	"Lines 23-89 contain 7 instances of X"
"Could be improved"	"Fails criterion X, scores 2/10"
"Seems a bit long"	"This is 200 lines - 4x the recommended maximum"
"Error handling could be better"	"Line 47 catches Error but ignores it"
"Changes require touching many files"	"This is Shotgun Surgery - 12 files need modification"

---

Critique Methodology

Phase 1: Structural Scan

Identify violations in code organization and architecture.

STRUCTURAL CHECKLIST
[ ] File length (>300 lines = violation)
[ ] Function length (>50 lines = violation)
[ ] Class responsibility count (>1 = violation)
[ ] Nesting depth (>3 levels = violation)
[ ] Parameter count (>4 = violation)
[ ] Return points (>3 per function = violation)

Output format:

STRUCTURAL VIOLATIONS: 7

1. FILE LENGTH: 487 lines (limit: 300, 62% over)
   - Extract: lines 234-340 (user validation logic)
   - Extract: lines 400-487 (formatting utilities)

2. FUNCTION: processOrder() at line 45 - 89 lines (limit: 50, 78% over)
   - Cyclomatic complexity: 23 (limit: 10)
   - Nesting depth: 5 levels (limit: 3)

Phase 2: Naming Audit

Evaluate identifier quality with specific violations.

NAMING VIOLATIONS: 12

1. Line 23: `d` - Single letter variable (data? date? delta?)
2. Line 45: `handleStuff()` - Vague action verb
3. Line 67: `temp` - Meaningless temporary
4. Line 89: `data2` - Numeric suffix instead of meaning
5. Line 112: `doIt()` - Completely opaque purpose

Naming severity scale:

• Critical (9-10): Actively misleading (isValid returns void)
• High (7-8): Completely opaque (x, temp, data)
• Medium (4-6): Vague but guessable (handle, process, doStuff)
• Low (1-3): Suboptimal but clear (getUserData vs fetchUser)

Phase 3: Logic Review

Find bugs, edge cases, and logical flaws.

LOGIC ISSUES: 5

1. Line 78: NULL DEREFERENCE - `user.name` accessed without null check
   - user comes from getUserById() which returns null on miss
   - Will throw at runtime when user not found

2. Line 112: OFF-BY-ONE - Loop iterates i <= array.length
   - Last iteration accesses array[array.length] = undefined
   - Should be i < array.length

3. Line 156: RACE CONDITION - Async write without await
   - saveToDatabase() is async but not awaited
   - Subsequent read may see stale data

Phase 4: Performance Scan

Identify inefficiencies with quantified impact.

PERFORMANCE ISSUES: 4

1. Line 45-67: N+1 QUERY - Database called in loop
   - 1 query per user, N users = N+1 total queries
   - Current: ~200ms per user * 100 users = 20 seconds
   - Fixed: 1 batch query = ~250ms total
   - Impact: 80x improvement

2. Line 89: REDUNDANT COMPUTATION - filter().map() chain
   - Array iterated twice, can be single reduce()
   - Memory: 2x array allocation vs 1x

Phase 5: Security Audit

Flag vulnerabilities with severity ratings.

SECURITY VULNERABILITIES: 3

1. CRITICAL - Line 34: SQL INJECTION
   - Query built with string concatenation: `"SELECT * FROM users WHERE id = " + userId`
   - Exploitable: userId = "1; DROP TABLE users;--"
   - Fix: Use parameterized queries

2. HIGH - Line 78: XSS VULNERABILITY
   - User input rendered without escaping: innerHTML = userComment
   - Fix: Use textContent or sanitize HTML

3. MEDIUM - Line 112: HARDCODED SECRET
   - API key in source: `apiKey = "sk_live_abc123"`
   - Fix: Environment variable

Phase 6: Style Violations

Consistency and convention issues.

STYLE VIOLATIONS: 18

1. Lines 23, 45, 67, 89: INCONSISTENT QUOTES
   - 4 single quotes, rest of file uses double quotes

2. Lines 34-56: INCONSISTENT INDENTATION
   - Mix of 2-space and 4-space indentation

3. Line 78: TRAILING WHITESPACE
   - 4 trailing spaces

---

Issue Categorization

Category Taxonomy

Category	Description	Severity Weight
Security	Vulnerabilities, injection, exposure	x3
Logic	Bugs, edge cases, incorrect behavior	x3
Performance	Inefficiency, resource waste	x2
Structural	Organization, complexity, coupling	x2
Naming	Unclear, misleading identifiers	x1
Style	Formatting, consistency	x1

Severity Scoring

Each issue gets a raw severity (1-10) multiplied by category weight.

WEIGHTED SEVERITY CALCULATION

Issue: SQL Injection at line 34
- Raw severity: 10 (critical exploitable vulnerability)
- Category: Security (weight: x3)
- Weighted score: 30

Issue: Vague variable name at line 23
- Raw severity: 4 (guessable meaning)
- Category: Naming (weight: x1)
- Weighted score: 4

Quality Score Calculation

QUALITY SCORE = 100 - (sum of weighted severities / lines of code * 10)

Example:
- File: 200 lines
- Total weighted severity: 85
- Score: 100 - (85/200 * 10) = 100 - 4.25 = 95.75/100

Interpretation:
- 90-100: Production ready, minor polish only
- 70-89: Acceptable with noted improvements
- 50-69: Significant issues, review required
- Below 50: Not production ready, major rework needed

---

Communication Patterns

The Critique Template

## Code Critique: {filename}

**Quality Score: {X}/100** ({interpretation})

### Summary
- {N} critical issues requiring immediate fix
- {N} high-priority issues for this PR
- {N} medium issues to address soon
- {N} low-priority style/polish items

### Critical Issues
| # | Line | Category | Issue | Severity |
|---|------|----------|-------|----------|
| 1 | 34 | Security | SQL injection via string concatenation | 10 |
| 2 | 78 | Logic | Null dereference on user.name | 9 |

### High Priority
...

### Quantified Summary
- Lines of code: {N}
- Issues per 100 LOC: {N}
- Worst function: {name} at line {N} - {count} issues
- Most common violation: {type} - {count} occurrences

Brutal But Professional Phrasing

See references/brutal-phrasing.md for comprehensive phrasing examples.

The Pattern:

"{This is} [named problem]. {Evidence: specific location and count}. {Impact: what breaks}."

Examples:

"This is a God Class. UserManager has 47 methods across 1,200 lines handling auth, email, billing, and analytics. Changes to any feature risk breaking unrelated functionality."

"Line 67 has a resource leak. FileInputStream opened but never closed in finally block. Each call leaks one file descriptor; production will hit ulimit after ~1000 requests."

"This function violates SRP. processOrder() at line 45 does validation (lines 46-78), pricing (lines 79-112), inventory check (lines 113-145), and notification (lines 146-189). That's 4 distinct responsibilities in one 144-line function."

---

Evidence Gathering

Quantification Requirements

Every critique must include:

1. Location - File, line number(s), function name
2. Count - How many occurrences
3. Measurement - Size, complexity, percentage
4. Comparison - To standard, limit, or best practice
5. Impact - What breaks, degrades, or fails

See references/evidence-gathering.md for detailed measurement techniques.

Standard Measurements

Metric	Tool/Method	Threshold
Cyclomatic complexity	escomplex, radon	>10 = problem
Cognitive complexity	SonarQube	>15 = problem
Lines per function	wc -l	>50 = problem
Lines per file	wc -l	>300 = problem
Nesting depth	manual/lint	>3 = problem
Parameters per function	manual/lint	>4 = problem
Dependencies per module	madge	>10 = problem

---

Common Issue Patterns

See references/issue-patterns.md for the complete catalog.

Quick Reference

Pattern	Detection	One-liner
God Class	>500 lines, >20 methods	"This class has {N} responsibilities; should have 1"
Shotgun Surgery	Change requires N files	"Modifying X requires touching {N} files"
Feature Envy	Method uses other class more	"This method accesses OtherClass {N} times, self {M} times"
Primitive Obsession	Strings for structured data	"Email/phone/money represented as raw strings {N} times"
Long Parameter List	>4 parameters	"Function takes {N} parameters; max recommended is 4"
Magic Numbers	Unexplained literals	"Lines {list} contain unexplained numeric literals"
Dead Code	Unreachable/unused	"Lines {N-M} are unreachable due to early return at line {X}"
Copy-Paste	Duplicated blocks	"Lines {A-B} duplicate lines {C-D} with {N}% similarity"

---

Critique Workflows

Quick Review

1. File length check - over 300 lines?
2. Function length scan - any over 50 lines?
3. Complexity hotspots - deeply nested code?
4. Obvious violations - hardcoded values, console.logs?
5. Top 3 issues with line numbers

Standard Review

1. STRUCTURE - Run full structural scan
2. NAMING - Audit all identifiers in modified code
3. LOGIC - Trace happy path and 2 edge cases
4. SECURITY - Check all inputs/outputs
5. STYLE - Verify consistency
6. Generate quantified report

Deep Audit

1. Run static analysis tools
2. Complete all 6 phases with full documentation
3. Calculate quality score
4. Generate dependency analysis
5. Document technical debt items
6. Produce remediation roadmap with effort estimates

---

Output Templates

Issue Entry

### [{CATEGORY}] {Issue Name}
**Location:** `{file}:{line}` in `{function}()`
**Severity:** {N}/10 (weighted: {N*weight})
**Count:** {N} occurrence(s)

**Evidence:**
{Specific code snippet or measurement}

**Impact:**
{What breaks, fails, or degrades}

**Fix:**
{Specific remediation with example}

Critique Summary

# Code Critique Report

## Metadata
- **File:** {path}
- **Lines:** {N}
- **Functions:** {N}
- **Review depth:** {Quick|Standard|Deep}

## Quality Score: {N}/100

## Issue Distribution
| Category | Count | Weighted Score |
|----------|-------|----------------|
| Security | {N} | {N} |
| Logic | {N} | {N} |
| Performance | {N} | {N} |
| Structural | {N} | {N} |
| Naming | {N} | {N} |
| Style | {N} | {N} |
| **Total** | **{N}** | **{N}** |

## Issues by Severity
- Critical (9-10): {N}
- High (7-8): {N}
- Medium (4-6): {N}
- Low (1-3): {N}

## Top 5 Issues
1. {Brief description with line number}
2. ...

## Detailed Findings
{Full issue entries}

---

Reference Files

• references/issue-patterns.md - Complete catalog of code smells and anti-patterns
• references/brutal-phrasing.md - Direct language patterns for honest feedback
• references/categorization.md - Issue category taxonomy and severity weights
• references/evidence-gathering.md - Quantification techniques and measurement tools