# Code Review Exercises

## Exercise 1: Write a PR Description

**Task:** Create a PR description for a hypothetical change: "Add input validation to the login form to reject empty username and password."

**Validation:**
- [ ] Contains "What" section describing the change
- [ ] Contains "Why" section explaining the rationale
- [ ] Contains "How" section with implementation notes
- [ ] Includes at least one testing note

**Hints:**
1. What = the concrete change (validation logic, error messages)
2. Why = security, UX, or compliance reason
3. How = where validation runs, what gets validated

---

## Exercise 2: Transform Vague Feedback

**Task:** A reviewer wrote: "This could be better." The code in question uses a nested `for` loop to find duplicates in an array. Rewrite this as specific, constructive feedback with a suggested alternative.

**Validation:**
- [ ] Identifies the specific issue (e.g., O(n²) complexity)
- [ ] Explains why it matters (performance, scalability)
- [ ] Suggests an alternative (e.g., Set, Map, or sort-then-compare)

**Hints:**
1. Vague feedback leaves the author guessing — be concrete
2. Explain the tradeoff (nested loops vs Set.has)
3. A code snippet or pseudocode helps

---

## Exercise 3: Prioritize Review Comments

**Task:** You received these five comments on a PR. Order them by priority (highest first) and label each: blocking, should-fix, or nit.

- "Use `const` instead of `let` for `config`"
- "The password is echoed in logs on line 89"
- "Consider extracting this into a helper function"
- "Possible race condition: two requests could overwrite the same file"
- "Typo: 'recieve' → 'receive'"

**Validation:**
- [ ] Security issue (password in logs) is highest or second-highest
- [ ] Race condition is blocking or should-fix
- [ ] Style (const, extract helper) is nit or lower priority
- [ ] Typo is nit

**Hints:**
1. Security and correctness trump style
2. Race conditions can cause data loss — treat seriously
3. Nits improve quality but don't block merge

---

## Exercise 4: Respond to Critical Feedback

**Task:** A senior reviewer wrote: "This approach won't scale. We'll have thousands of users and this does a full table scan per request." Your implementation uses `SELECT * FROM users WHERE ...` without an index.

Write a constructive response (2–4 sentences) that:
1. Acknowledges the concern
2. Shows you understand the problem
3. Proposes or asks about a fix (index, caching, or different query)

**Validation:**
- [ ] No defensive tone ("it works for now")
- [ ] Demonstrates understanding of the scalability issue
- [ ] Proposes a concrete next step or asks for guidance

**Hints:**
1. "You're right" or "Good catch" shows receptiveness
2. Indexes, caching, or pagination are common fixes
3. Asking "Should I add an index on X?" is collaborative

---

## Exercise 5: Create a Review Checklist

**Task:** Build a 6-item checklist for code reviews you perform. Include at least one item from each category: correctness, readability, security, tests, and process.

**Validation:**
- [ ] At least 6 items
- [ ] Covers correctness (e.g., edge cases, error handling)
- [ ] Covers readability (naming, structure)
- [ ] Covers security (inputs, secrets)
- [ ] Covers tests (new code has tests)
- [ ] Covers process (PR description, scope)

**Hints:**
1. "Does the PR description explain the goal?" is a good process item
2. "Are new code paths covered by tests?" ensures quality
3. "Any hardcoded secrets or credentials?" is a security staple