Code Quality

Code Quality Management

Comprehensive skill for improving code quality through two-stage review (spec compliance first, then code quality), surgical refactoring, and self-evaluation loops.

• Leverage native parallel subagent dispatch and 200k+ context windows where available.

Activation Conditions

Use symptom -> action triggers: when one matches, apply this skill and verify with the protocol below.

two-stage review (spec compliance first, then code quality):

• Performing two-stage reviews (spec compliance first, then code quality), analyzing pull requests
• Checking code quality, security auditing, performance reviews
• Examining code for bugs, vulnerabilities, best practices violations
• "Review code", "check for issues", "audit code", "analyze PR"

Refactoring:

• Code is hard to understand or maintain
• Functions/classes are too large, code smells need addressing
• Adding features is difficult due to code structure
• User asks "clean up this code", "refactor this", "improve this"

Self-Evaluation:

• Implementing self-critique and reflection loops for agent outputs
• Building evaluator-optimizer pipelines for quality-critical generation
• Creating test-driven code refinement workflows
• Designing rubric-based or LLM-as-judge evaluation systems
• Adding iterative improvement to agent outputs (code, reports, analysis)
• Measuring and improving agent response quality

Part 1: two-stage review (spec compliance first, then code quality)

Review Priorities

When performing a two-stage review (spec compliance first, then code quality), prioritize issues in this order:

🔴 CRITICAL (Block merge)

• Security: Vulnerabilities, exposed secrets, authentication/authorization issues
• Correctness: Logic errors, data corruption risks, race conditions
• Breaking Changes: API contract changes without versioning
• Data Loss: Risk of data loss or corruption

🟡 IMPORTANT (Requires discussion)

• Code Quality: Severe violations of SOLID principles, excessive duplication
• Test Coverage: Missing tests for critical paths or new functionality
• Performance: Obvious performance bottlenecks (N+1 queries, memory leaks)
• Architecture: Significant deviations from established patterns

🟢 SUGGESTION (Non-blocking improvements)

• Readability: Poor naming, complex logic that could be simplified
• Optimization: Performance improvements without functional impact
• Best Practices: Minor deviations from conventions
• Documentation: Missing or incomplete comments/documentation

Review Principles

1. Be specific: Reference exact lines, files, and provide concrete examples 2. Provide context: Explain WHY something is an issue and potential impact 3. Suggest solutions: Show corrected code when applicable, not just what's wrong 4. Be constructive: Focus on improving code, not criticizing the author 5. Recognize good practices: Acknowledge well-written code and smart solutions 6. Be pragmatic: Not every suggestion needs immediate implementation 7. Group related comments: Avoid multiple comments about the same topic

Part 2: Refactoring

The Golden Rules

1. Behavior is preserved - Refactoring doesn't change what code does, only how 2. Small steps - Make tiny changes, test after each 3. Version control is your friend - Commit before and after each safe state 4. Tests are essential - Without tests, you're not refactoring, you're editing 5. One thing at a time - Don't mix refactoring with feature changes

When NOT to Refactor

• Code that works and won't change again (if it ain't broke...)
• Critical production code without tests (add tests first)
• When you're under a tight deadline
• "Just because" - need a clear purpose

Refactoring Techniques

Extract Method

// Before
function processOrder(order) {
    if (order.status === 'pending') {
        // 20 lines of validation logic
        // 15 lines of calculation logic
        // 10 lines of notification logic
    }
}

// After
function processOrder(order) {
    if (order.status === 'pending') {
        validateOrder(order);
        calculateTotals(order);
        sendNotification(order);
    }
}

Rename Variable/Function

Use meaningful names that describe purpose:

// Before
const d = new Date();
process(v, u);

// After
const currentDate = new Date();
processValidation(validatedValue, userId);

Extract Class

// Before
function calculateCartTotal(cart, user, shippingMethod, taxRate) {
    // Complex logic mixing user details, cart items, shipping, tax
}

// After
class OrderCalculator {
    constructor(cart, user) {
        this.cart = cart;
        this.user = user;
    }

    calculate(shippingMethod, taxRate) {
        const subtotal = this.calculateSubtotal();
        const shipping = this.calculateShipping(shippingMethod);
        const tax = this.calculateTax(taxRate);
        return subtotal + shipping + tax;
    }
}

Common Code Smells and Fixes

Long Method

Problem: Methods longer than 30-50 lines Fix: Extract smaller, focused methods

Duplicate Code

Problem: Same logic in multiple places Fix: Extract to shared function/method

Large Class

Problem: Classes with too many responsibilities Fix: Extract smaller, focused classes

Magic Numbers

Problem: Unnamed numeric literals

// Before
if (status > 3) { ... }

// After
const MAX_PENDING_DURATION_DAYS = 3;
if (status > MAX_PENDING_DURATION_DAYS) { ... }

Feature Envy

Problem: Method uses data from another class more than its own Fix: Move method to class it's envious of

---

Part 3: Self-Evaluation Patterns

Pattern 1: Basic Reflection

Agent evaluates and improves its own output through self-critique.

def reflect_and_refine(task: str, criteria: list[str], max_iterations: int = 3) -> str:
    """Generate with reflection loop."""
    output = llm(f"Complete this task:\n{task}")

    for i in range(max_iterations):
        # Self-critique
        critique = llm(f"""
        Evaluate this output against criteria: {criteria}
        Output: {output}
        Rate each: PASS/FAIL with feedback as JSON.
        """)

        critique_data = json.loads(critique)
        all_pass = all(c["status"] == "PASS" for c in critique_data.values())
        if all_pass:
            return output

        # Refine based on critique
        failed = {k: v["feedback"] for k, v in critique_data.items() if v["status"] == "FAIL"}
        output = llm(f"Improve to address: {failed}\nOriginal: {output}")

    return output

Key insight: Use structured JSON output for reliable parsing of critique results.

Pattern 2: Evaluator-Optimizer

Separate generation and evaluation into distinct components for clearer responsibilities.

class EvaluatorOptimizer:
    def __init__(self, score_threshold: float = 0.8):
        self.score_threshold = score_threshold

    def generate(self, task: str) -> str:
        return llm(f"Complete: {task}")

    def evaluate(self, output: str, task: str) -> dict:
        return json.loads(llm(f"""
        Evaluate output for task: {task}
        Output: {output}
        Return JSON: {{"overall_score": 0-1, "dimensions": {{"accuracy": ..., "clarity": ...}}}
        """))

    def optimize(self, output: str, feedback: dict) -> str:
        return llm(f"Improve based on feedback: {feedback}\nOutput: {output}")

    def run(self, task: str, max_iterations: int = 3) -> str:
        output = self.generate(task)
        for _ in range(max_iterations):
            evaluation = self.evaluate(output, task)
            if evaluation["overall_score"] >= self.score_threshold:
                break
            output = self.optimize(output, evaluation)
        return output

Pattern 3: Code-Specific Reflection

Test-driven refinement loop for code generation.

class CodeReflector:
    def reflect_and_fix(self, spec: str, max_iterations: int = 3) -> str:
        code = llm(f"Write Python code for: {spec}")
        tests = llm(f"Generate pytest tests for: {spec}\nCode: {code}")

        for _ in range(max_iterations):
            result = run_tests(code, tests)
            if result["success"]:
                return code
            code = llm(f"Fix error: {result['error']}\nCode: {code}")
        return code

Evaluation Strategies

Outcome-Based

Evaluate whether output achieves expected result.

def evaluate_outcome(task: str, output: str, expected: str) -> str:
    return llm(f"Does output achieve expected outcome? Task: {task}, Expected: {expected}, Output: {output}")

LLM-as-Judge

Use LLM to compare and rank outputs.

def llm_judge(output_a: str, output_b: str, criteria: str) -> str:
    return llm(f"Compare outputs A and B for {criteria}. Which is better and why?")

Rubric-Based

Score outputs against weighted dimensions.

RUBRIC = {
    "accuracy": {"weight": 0.4},
    "clarity": {"weight": 0.3},
    "completeness": {"weight": 0.3}
}

def evaluate_with_rubric(output: str, rubric: dict) -> float:
    scores = json.loads(llm(f"Rate 1-5 for each dimension: {list(rubric.keys())}\nOutput: {output}"))
    return sum(scores[d] * rubric[d]["weight"] for d in rubric) / 5

---

Anti-Patterns

• Starting work before the plan or gate is clear: Execution drifts when success criteria are implied instead of explicit.
• Treating verification as optional cleanup: The last mile is where regressions and missing updates are usually hiding.
• Mixing planning, implementation, and release work in one jump: You lose the causal chain that explains why a change is safe.

Verification Protocol

Before claiming "skill applied successfully":

1. Pass/fail: The Code Quality workflow starts from explicit success criteria, constraints, and stop conditions. 2. Pass/fail: Required evidence is collected before any completion, approval, or readiness claim. 3. Pass/fail: The next action follows the documented gate order without skipping review or verification steps. 4. Pressure-test scenario: Apply the workflow under time pressure with one failing check and one tempting shortcut. 5. Success metric: Zero rationalizations; blocked, failed, or unverified work is reported as such.

Multi-Language Review Examples

Python

# Before
def approve(order, notifier):
    if order.total > 1000:
        notifier.send(order.customer_email, order.total)
    return order.total

# After
def calculate_total(order: Order) -> int:
    return order.total

def notify_high_value_order(order: Order, notifier: Notifier) -> None:
    if order.total > HIGH_VALUE_THRESHOLD:
        notifier.send(order.customer_email, order.total)

C#

// Before
public decimal Process(Order order)
{
    if (order.Total > 1000) _email.Send(order.CustomerEmail, order.Total);
    return order.Total;
}

// After
public decimal CalculateTotal(Order order) => order.Total;

public void NotifyHighValueCustomer(Order order)
{
    if (order.Total > HighValueThreshold)
    {
        _email.Send(order.CustomerEmail, order.Total);
    }
}

Java

// Before
BigDecimal process(Order order) {
    if (order.total().compareTo(THRESHOLD) > 0) {
        email.send(order.customerEmail(), order.total());
    }
    return order.total();
}

// After
BigDecimal calculateTotal(Order order) {
    return order.total();
}

void notifyHighValueCustomer(Order order) {
    if (order.total().compareTo(THRESHOLD) > 0) {
        email.send(order.customerEmail(), order.total());
    }
}

Go

// Before
func Process(order Order, notifier Notifier) int {
    if order.Total > highValueThreshold {
        notifier.Send(order.CustomerEmail, order.Total)
    }
    return order.Total
}

// After
func CalculateTotal(order Order) int {
    return order.Total
}

func NotifyHighValueCustomer(order Order, notifier Notifier) {
    if order.Total > highValueThreshold {
        notifier.Send(order.CustomerEmail, order.Total)
    }
}

AI-Generated Code Specific Checks

• Hallucinated APIs or options: Verify every imported type, method, CLI flag, and config field against the real dependency version before trusting the sample.
• Inconsistent style drift: AI often mixes naming, file structure, or error-handling styles from different codebases, so compare the output against local conventions before merging.
• Over-engineering for a simple requirement: Generated code commonly adds abstractions, wrappers, or extension points that the current task does not need.
• Hidden edge-case gaps: AI can produce convincing happy-path logic while skipping null handling, retries, authorization checks, or cleanup paths.

Automated Tooling Integration

ESLint and Prettier

{
  "scripts": {
    "lint": "eslint . --ext .js,.jsx,.ts,.tsx --max-warnings=0",
    "format": "prettier --write .",
    "format:check": "prettier --check ."
  }
}

SonarQube Scan

- name: SonarQube scan
  run: |
    sonar-scanner \
      -Dsonar.projectKey=my-app \
      -Dsonar.sources=src \
      -Dsonar.tests=tests \
      -Dsonar.javascript.lcov.reportPaths=coverage/lcov.info

CI Quality Gates

Use CI quality gates to enforce linting, formatting, test coverage, and static-analysis thresholds before review or merge.

jobs:
  quality:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: actions/setup-node@v4
        with:
          node-version: 22
      - run: npm ci
      - run: npm run lint
      - run: npm run format:check
      - run: npm test -- --coverage
      - run: sonar-scanner

Use the gate to fail fast on lint errors, formatting drift, coverage regressions, and maintainability warnings before review starts.

Best Practices

For two-stage reviews (spec compliance first, then code quality)

• Focus on code behavior, not personal style preferences
• Provide actionable feedback with examples
• Balance critique with recognition of good work
• Consider project context and constraints

For Refactoring

• Always have tests before refactoring
• Commit frequently to maintain safety
• Keep changes small and verifiable
• Document non-obvious refactoring decisions

For Self-Evaluation

• Define clear, measurable evaluation criteria upfront
• Set iteration limits (3-5) to prevent infinite loops
• Add convergence detection if scores aren't improving
• Log full iteration trajectory for debugging and analysis
• Use structured output (JSON) for reliable parsing

---

Quality Improvement Checklist

two-stage review (spec compliance first, then code quality) Checklist

## two-stage review (spec compliance first, then code quality) Assessment

### Functionality
- [ ] Logic is correct and achieves intended purpose
- [ ] Edge cases are handled appropriately
- [ ] Error handling is comprehensive
- [ ] No obvious bugs or race conditions

### Code Quality
- [ ] Code is readable and maintainable
- [ ] Naming is descriptive and consistent
- [ ] Functions/classes have single responsibility
- [ ] No unnecessary complexity or obfuscation

### Architecture
- [ ] Follows established project patterns
- [ ] Appropriate use of design patterns
- [ ] Proper separation of concerns
- [ ] No tight coupling or hidden dependencies

Refactoring Checklist

## Refactoring Safety Checklist

### Pre-Refactoring
- [ ] Tests exist and pass
- [ ] Version control branch is clean
- [ ] Understand current behavior thoroughly

### During Refactoring
- [ ] Making small, incremental changes
- [ ] Running tests after each change
- [ ] Committing each working intermediate state
- [ ] Preserving external behavior

### Post-Refactoring
- [ ] All tests still pass
- [ ] Code is simpler and clearer
- [ ] No new bugs introduced
- [ ] Documentation updated if needed

Self-Evaluation Checklist

## Evaluation Implementation Checklist

### Setup
- [ ] Define evaluation criteria/rubric
- [ ] Set score threshold for "good enough"
- [ ] Configure max iterations (default: 3)

### Implementation
- [ ] Implement generate() function
- [ ] Implement evaluate() function with structured output
- [ ] Implement optimize() function
- [ ] Wire up to refinement loop

### Safety
- [ ] Add convergence detection
- [ ] Log all iterations for debugging
- [ ] Handle evaluation parse failures gracefully

---

## References & Resources

### Documentation
- [Refactoring Catalog](./references/refactoring-catalog.md) — 12 refactoring techniques with before/after code examples and pitfalls
- [Code Smells](./references/code-smells.md) — 17 code smells organized by category with detection signals and remedies

### Scripts
- [Review Checklist](./scripts/review-checklist.py) — Python script for automated static analysis of JS/TS files

### Examples
- [Refactoring Walkthrough](./examples/refactoring-walkthrough.md) — Step-by-step React component refactoring from 160 lines to clean architecture

---

<!-- PORTABILITY:START -->
## Cross-Client Portability

This skill is written to stay usable across GitHub Copilot, Claude Code, Codex, and Gemini CLI.

- GitHub Copilot: keep the folder in a Copilot-visible skill or plugin path, or wrap the workflow as project instructions if the host does not support portable skill folders directly.
- Claude Code: keep the folder in a local skills directory or a compatible plugin or marketplace source.
- Codex: install or sync the folder into `$CODEX_HOME/skills/<skill-name>` and restart Codex after major changes.
- Gemini CLI: this repository generates a project command named `/skills:code-quality` from this skill. Rebuild commands with `python scripts/export-gemini-skill.py code-quality` and then run `/commands reload` inside Gemini CLI.

<!-- PORTABILITY:END -->

<!-- MCP:START -->
## MCP Availability And Fallback

Preferred MCP Server: None required

- Fallback prompt: "Use the Code Quality Management skill without MCP. Rely on the local `SKILL.md`, bundled references or scripts, and manual verification. Show the exact commands, evidence, and final checks you used before concluding."
- If the current host does not expose a matching server, use the bundled references, scripts, native toolchain, and manual workflow already described in this skill.
- Treat direct local verification, rendered output, logs, tests, or screenshots as the fallback evidence path before completion.

<!-- MCP:END -->

## Related Skills

- [development-workflow](../development-workflow/SKILL.md): Use it when the workflow also needs planning, quality gates, and delivery tracking.
- [systematic-debugging](../systematic-debugging/SKILL.md): Use it when the workflow also needs root-cause debugging before proposing fixes.
- [test-driven-development](../test-driven-development/SKILL.md): Use it when the workflow also needs test-first implementation and regression safety.
- [verification-before-completion](../verification-before-completion/SKILL.md): Use it when the workflow also needs final evidence checks before claiming completion.

Changelog

[2026-04-25] - Version 1.2 Verification Protocol Refresh

Added

• Added a Verification Protocol section with skill-specific pass/fail checks, one pressure-test scenario, and a measurable success metric.
• Added guidance to leverage native parallel subagent dispatch and 200k+ context windows where available.

Changed

• Updated SKILL.md frontmatter to version: "1.2" and last_updated: 2026-04-25.
• Reframed activation guidance toward symptom -> action triggers and standardized two-stage review wording where applicable.

[2026-04-24] - Version 1.1 Refresh

Changed

• Updated the SKILL frontmatter version to 1.1 for the 2026-04-24 catalog refresh.

[2026-04-24] - Post-Refresh Cleanup

Fixed

• Clarified the CI quality-gates heading and reformatted the SonarQube example so the automated tooling section is copy-paste ready.

[2026-04-24] - Skill Refresh

Changed

• Standardized the SKILL frontmatter with version metadata, last-updated date, tags, and a concise catalog description.
• Reformatted the portability and MCP guidance with a preferred server line, a copy-paste fallback prompt, and consistent bullet lists.
• Added a catalog-standard Anti-Patterns section and refreshed the Related Skills links at the end of the skill.
• Added multi-language review examples for Python, C#, Java, and Go, plus AI-generated-code checks and tooling integration samples for ESLint, Prettier, SonarQube, and CI quality gates.

[2026-04-24] - Catalog Audit Cleanup

Fixed

• Removed obsolete standalone Skill Paths guidance that duplicated the generated portability section.

All notable changes to this skill will be documented in this file.

[2026-04-04] - Cross-Client Portability Refresh

Changed

• Added a standard portability note covering GitHub Copilot, Claude Code, Codex, and Gemini CLI.
• Clarified that the core workflow does not require a dedicated MCP server and can run with local tools alone.

Tested

• Validated SKILL.md frontmatter, portability sections, and Gemini export readiness with python scripts/validate-skills.py.

[2026-03-09] - Workspace Modernization

Changed

• Removed duplicated related-skill content from SKILL.md to reduce noise

[2026-02-28] — Description Rewrite & Cross-References

Changed

• Rewrote skill description to ~200 characters with clear, specific activation keywords
• Improved keyword specificity to reduce overlap with related skills

Added

• ## Related Skills cross-reference table with 2-4 related skills and "Use When" guidance

Refactoring Walkthrough: React Component

A step-by-step refactoring of a realistic React component. We start with a messy component full of code smells and transform it into clean, maintainable code through incremental improvements.

---

The Starting Point: ProductPage.jsx

This component displays a product, handles cart operations, manages reviews, and tracks analytics. It's 160+ lines with multiple code smells.

import { useState, useEffect } from "react";

export default function ProductPage({ productId, user }) {
  const [product, setProduct] = useState(null);
  const [loading, setLoading] = useState(true);
  const [error, setError] = useState(null);
  const [reviews, setReviews] = useState([]);
  const [reviewText, setReviewText] = useState("");
  const [reviewRating, setReviewRating] = useState(5);
  const [cartCount, setCartCount] = useState(0);
  const [showSuccess, setShowSuccess] = useState(false);
  const [selectedSize, setSelectedSize] = useState(null);
  const [selectedColor, setSelectedColor] = useState(null);
  const [isFavorite, setIsFavorite] = useState(false);
  const [quantity, setQuantity] = useState(1);

  useEffect(() => {
    setLoading(true);
    fetch(`https://api.example.com/products/${productId}`)
      .then((res) => {
        if (!res.ok) throw new Error("Failed to fetch");
        return res.json();
      })
      .then((data) => {
        setProduct(data);
        setSelectedSize(data.sizes[0]);
        setSelectedColor(data.colors[0]);
        setLoading(false);
      })
      .catch((err) => {
        setError(err.message);
        setLoading(false);
      });
  }, [productId]);

  useEffect(() => {
    fetch(`https://api.example.com/products/${productId}/reviews`)
      .then((res) => res.json())
      .then((data) => setReviews(data))
      .catch((err) => console.log(err));
  }, [productId]);

  useEffect(() => {
    if (user) {
      fetch(`https://api.example.com/users/${user.id}/favorites`)
        .then((res) => res.json())
        .then((favs) => {
          setIsFavorite(favs.some((f) => f.productId === productId));
        })
        .catch((err) => console.log(err));
    }
  }, [productId, user]);

  function handleAddToCart() {
    if (!selectedSize) {
      alert("Please select a size");
      return;
    }
    if (!selectedColor) {
      alert("Please select a color");
      return;
    }
    fetch("https://api.example.com/cart", {
      method: "POST",
      headers: { "Content-Type": "application/json" },
      body: JSON.stringify({
        productId: productId,
        size: selectedSize,
        color: selectedColor,
        quantity: quantity,
        price: product.price,
      }),
    })
      .then((res) => res.json())
      .then((data) => {
        setCartCount(data.totalItems);
        setShowSuccess(true);
        setTimeout(() => setShowSuccess(false), 3000);
        // analytics
        if (window.gtag) {
          window.gtag("event", "add_to_cart", {
            item_id: productId,
            item_name: product.name,
            price: product.price,
            quantity: quantity,
          });
        }
      })
      .catch((err) => {
        console.log(err);
        alert("Failed to add to cart");
      });
  }

  function handleSubmitReview() {
    if (reviewText.length < 10) {
      alert("Review must be at least 10 characters");
      return;
    }
    if (!user) {
      alert("Please log in to submit a review");
      return;
    }
    fetch(`https://api.example.com/products/${productId}/reviews`, {
      method: "POST",
      headers: { "Content-Type": "application/json" },
      body: JSON.stringify({
        text: reviewText,
        rating: reviewRating,
        userId: user.id,
        userName: user.name,
      }),
    })
      .then((res) => res.json())
      .then((newReview) => {
        setReviews([newReview, ...reviews]);
        setReviewText("");
        setReviewRating(5);
      })
      .catch((err) => {
        console.log(err);
        alert("Failed to submit review");
      });
  }

  function handleToggleFavorite() {
    const method = isFavorite ? "DELETE" : "POST";
    fetch(`https://api.example.com/users/${user.id}/favorites/${productId}`, {
      method: method,
    })
      .then(() => setIsFavorite(!isFavorite))
      .catch((err) => console.log(err));
  }

  if (loading) return <div className="loading">Loading...</div>;
  if (error) return <div className="error">Error: {error}</div>;
  if (!product) return <div>Product not found</div>;

  // calculate average rating
  let totalRating = 0;
  for (let i = 0; i < reviews.length; i++) {
    totalRating += reviews[i].rating;
  }
  const avgRating = reviews.length > 0 ? totalRating / reviews.length : 0;

  // calculate discounted price
  let finalPrice = product.price;
  if (product.discount > 0) {
    finalPrice = product.price * (1 - product.discount / 100);
  }

  return (
    <div className="product-page">
      <div className="product-header">
        <img src={product.image} alt={product.name} className="product-image" />
        <div className="product-info">
          <h1>{product.name}</h1>
          <div className="rating">
            {[1, 2, 3, 4, 5].map((star) => (
              <span key={star} className={star <= Math.round(avgRating) ? "star filled" : "star"}>
                ★
              </span>
            ))}
            <span>({reviews.length} reviews)</span>
          </div>
          <div className="price">
            {product.discount > 0 && (
              <span className="original-price">${product.price.toFixed(2)}</span>
            )}
            <span className="final-price">${finalPrice.toFixed(2)}</span>
            {product.discount > 0 && (
              <span className="discount-badge">{product.discount}% OFF</span>
            )}
          </div>
          <p className="description">{product.description}</p>

          <div className="options">
            <div className="size-selector">
              <label>Size:</label>
              {product.sizes.map((size) => (
                <button
                  key={size}
                  className={size === selectedSize ? "option selected" : "option"}
                  onClick={() => setSelectedSize(size)}
                >
                  {size}
                </button>
              ))}
            </div>
            <div className="color-selector">
              <label>Color:</label>
              {product.colors.map((color) => (
                <button
                  key={color}
                  className={color === selectedColor ? "option selected" : "option"}
                  onClick={() => setSelectedColor(color)}
                >
                  {color}
                </button>
              ))}
            </div>
          </div>

          <div className="actions">
            <input
              type="number"
              min="1"
              max="10"
              value={quantity}
              onChange={(e) => setQuantity(Number(e.target.value))}
            />
            <button onClick={handleAddToCart} className="add-to-cart">
              Add to Cart ({cartCount})
            </button>
            <button onClick={handleToggleFavorite} className="favorite">
              {isFavorite ? "♥ Favorited" : "♡ Favorite"}
            </button>
          </div>
          {showSuccess && <div className="success-message">Added to cart!</div>}
        </div>
      </div>

      <div className="reviews-section">
        <h2>Reviews ({reviews.length})</h2>
        {user && (
          <div className="review-form">
            <select value={reviewRating} onChange={(e) => setReviewRating(Number(e.target.value))}>
              {[5, 4, 3, 2, 1].map((r) => (
                <option key={r} value={r}>{r} Stars</option>
              ))}
            </select>
            <textarea
              value={reviewText}
              onChange={(e) => setReviewText(e.target.value)}
              placeholder="Write your review (min 10 characters)..."
            />
            <button onClick={handleSubmitReview}>Submit Review</button>
          </div>
        )}
        <div className="review-list">
          {reviews.map((review) => (
            <div key={review.id} className="review-card">
              <div className="review-header">
                <strong>{review.userName}</strong>
                <span>{[1, 2, 3, 4, 5].map((s) => (
                  <span key={s} className={s <= review.rating ? "star filled" : "star"}>★</span>
                ))}</span>
              </div>
              <p>{review.text}</p>
              <span className="review-date">
                {new Date(review.createdAt).toLocaleDateString()}
              </span>
            </div>
          ))}
        </div>
      </div>
    </div>
  );
}

Identified Code Smells

---

Step 1: Extract API Service

Move all fetch calls into a dedicated module. This removes Feature Envy and separates concerns.

Create `api/productApi.js`:

const API_BASE = "https://api.example.com";

export async function fetchProduct(productId) {
  const res = await fetch(`${API_BASE}/products/${productId}`);
  if (!res.ok) throw new Error(`Failed to fetch product: ${res.status}`);
  return res.json();
}

export async function fetchReviews(productId) {
  const res = await fetch(`${API_BASE}/products/${productId}/reviews`);
  if (!res.ok) throw new Error(`Failed to fetch reviews: ${res.status}`);
  return res.json();
}

export async function submitReview(productId, review) {
  const res = await fetch(`${API_BASE}/products/${productId}/reviews`, {
    method: "POST",
    headers: { "Content-Type": "application/json" },
    body: JSON.stringify(review),
  });
  if (!res.ok) throw new Error(`Failed to submit review: ${res.status}`);
  return res.json();
}

export async function addToCart(item) {
  const res = await fetch(`${API_BASE}/cart`, {
    method: "POST",
    headers: { "Content-Type": "application/json" },
    body: JSON.stringify(item),
  });
  if (!res.ok) throw new Error(`Failed to add to cart: ${res.status}`);
  return res.json();
}

export async function fetchFavorites(userId) {
  const res = await fetch(`${API_BASE}/users/${userId}/favorites`);
  if (!res.ok) throw new Error(`Failed to fetch favorites: ${res.status}`);
  return res.json();
}

export async function toggleFavorite(userId, productId, isFavorite) {
  const method = isFavorite ? "DELETE" : "POST";
  const res = await fetch(`${API_BASE}/users/${userId}/favorites/${productId}`, { method });
  if (!res.ok) throw new Error(`Failed to toggle favorite: ${res.status}`);
}

Impact: All 6 inline fetch calls removed from the component. API base URL defined once. Error handling is consistent.

---

Step 2: Extract Custom Hook — useProduct

Extract the product-loading logic into a reusable hook. This addresses Long Method and separates data-fetching from rendering.

Create `hooks/useProduct.js`:

import { useState, useEffect } from "react";
import { fetchProduct, fetchReviews, fetchFavorites } from "../api/productApi";

export function useProduct(productId, userId) {
  const [product, setProduct] = useState(null);
  const [reviews, setReviews] = useState([]);
  const [isFavorite, setIsFavorite] = useState(false);
  const [loading, setLoading] = useState(true);
  const [error, setError] = useState(null);

  useEffect(() => {
    let cancelled = false;

    async function load() {
      setLoading(true);
      setError(null);
      try {
        const [productData, reviewsData] = await Promise.all([
          fetchProduct(productId),
          fetchReviews(productId),
        ]);
        if (cancelled) return;
        setProduct(productData);
        setReviews(reviewsData);
      } catch (err) {
        if (!cancelled) setError(err.message);
      } finally {
        if (!cancelled) setLoading(false);
      }
    }

    load();
    return () => { cancelled = true; };
  }, [productId]);

  useEffect(() => {
    if (!userId) return;
    let cancelled = false;

    fetchFavorites(userId)
      .then((favs) => {
        if (!cancelled) {
          setIsFavorite(favs.some((f) => f.productId === productId));
        }
      })
      .catch(() => {}); // favorite status is non-critical

    return () => { cancelled = true; };
  }, [productId, userId]);

  function addReview(review) {
    setReviews((prev) => [review, ...prev]);
  }

  return { product, reviews, isFavorite, setIsFavorite, addReview, loading, error };
}

What improved:

• Race condition handled with cancelled flag
• Parallel data fetching with Promise.all
• Cleanup on unmount prevents state updates after unmount
• Review list update encapsulated in addReview

---

Step 3: Extract Sub-Components

Break the monolithic render into focused components. This addresses Large Class and Duplicated Code.

Create `components/StarRating.jsx`:

const STARS = [1, 2, 3, 4, 5];

export default function StarRating({ rating, count }) {
  const rounded = Math.round(rating);
  return (
    <div className="rating">
      {STARS.map((star) => (
        <span key={star} className={star <= rounded ? "star filled" : "star"}>
          ★
        </span>
      ))}
      {count != null && <span>({count} reviews)</span>}
    </div>
  );
}

The duplicated star-rendering logic (header + review cards) is now a single component.

Create `components/OptionSelector.jsx`:

export default function OptionSelector({ label, options, selected, onSelect }) {
  return (
    <div className="option-selector">
      <label>{label}:</label>
      {options.map((option) => (
        <button
          key={option}
          className={option === selected ? "option selected" : "option"}
          onClick={() => onSelect(option)}
        >
          {option}
        </button>
      ))}
    </div>
  );
}

Eliminates duplication between size selector and color selector.

Create `components/PriceDisplay.jsx`:

export default function PriceDisplay({ price, discountPercent }) {
  const finalPrice = discountPercent > 0 ? price * (1 - discountPercent / 100) : price;

  return (
    <div className="price">
      {discountPercent > 0 && (
        <span className="original-price">${price.toFixed(2)}</span>
      )}
      <span className="final-price">${finalPrice.toFixed(2)}</span>
      {discountPercent > 0 && (
        <span className="discount-badge">{discountPercent}% OFF</span>
      )}
    </div>
  );
}

Price calculation logic is encapsulated. The magic number 100 is now contextually clear inside a discountPercent computation.

Create `components/ReviewCard.jsx`:

import StarRating from "./StarRating";

export default function ReviewCard({ review }) {
  return (
    <div className="review-card">
      <div className="review-header">
        <strong>{review.userName}</strong>
        <StarRating rating={review.rating} />
      </div>
      <p>{review.text}</p>
      <time className="review-date">
        {new Date(review.createdAt).toLocaleDateString()}
      </time>
    </div>
  );
}

---

Step 4: Extract Review Form with Proper Validation

Replace alert() with inline validation state and extract the form into its own component.

Create `components/ReviewForm.jsx`:

import { useState } from "react";
import { submitReview } from "../api/productApi";

const MIN_REVIEW_LENGTH = 10;
const RATING_OPTIONS = [5, 4, 3, 2, 1];

export default function ReviewForm({ productId, user, onReviewAdded }) {
  const [text, setText] = useState("");
  const [rating, setRating] = useState(5);
  const [submitting, setSubmitting] = useState(false);
  const [error, setError] = useState(null);

  const validationError =
    text.length > 0 && text.length < MIN_REVIEW_LENGTH
      ? `Review must be at least ${MIN_REVIEW_LENGTH} characters`
      : null;

  const canSubmit = text.length >= MIN_REVIEW_LENGTH && !submitting;

  async function handleSubmit(e) {
    e.preventDefault();
    setSubmitting(true);
    setError(null);

    try {
      const newReview = await submitReview(productId, {
        text,
        rating,
        userId: user.id,
        userName: user.name,
      });
      onReviewAdded(newReview);
      setText("");
      setRating(5);
    } catch (err) {
      setError(err.message);
    } finally {
      setSubmitting(false);
    }
  }

  return (
    <form className="review-form" onSubmit={handleSubmit}>
      <select value={rating} onChange={(e) => setRating(Number(e.target.value))}>
        {RATING_OPTIONS.map((r) => (
          <option key={r} value={r}>{r} Stars</option>
        ))}
      </select>
      <textarea
        value={text}
        onChange={(e) => setText(e.target.value)}
        placeholder={`Write your review (min ${MIN_REVIEW_LENGTH} characters)...`}
      />
      {validationError && <p className="validation-error">{validationError}</p>}
      {error && <p className="error-message">{error}</p>}
      <button type="submit" disabled={!canSubmit}>
        {submitting ? "Submitting..." : "Submit Review"}
      </button>
    </form>
  );
}

What improved:

• alert() replaced with inline validation messages
• Loading state during submission
• Error state displayed in UI instead of alert
• Magic number 10 replaced with named constant
• Uses <form> with onSubmit instead of button onClick

---

Step 5: Extract Cart Logic with Analytics Separation

Separate analytics tracking from cart operations. Replace alert() and console.log().

Create `hooks/useCart.js`:

import { useState, useCallback } from "react";
import { addToCart as addToCartApi } from "../api/productApi";
import { trackAddToCart } from "../analytics";

const SUCCESS_MESSAGE_DURATION_MS = 3000;

export function useCart() {
  const [cartCount, setCartCount] = useState(0);
  const [showSuccess, setShowSuccess] = useState(false);
  const [error, setError] = useState(null);
  const [adding, setAdding] = useState(false);

  const addItem = useCallback(async (item, productMeta) => {
    setAdding(true);
    setError(null);

    try {
      const data = await addToCartApi(item);
      setCartCount(data.totalItems);
      setShowSuccess(true);
      setTimeout(() => setShowSuccess(false), SUCCESS_MESSAGE_DURATION_MS);
      trackAddToCart(productMeta);
    } catch (err) {
      setError(err.message);
    } finally {
      setAdding(false);
    }
  }, []);

  return { cartCount, showSuccess, error, adding, addItem };
}

Create `analytics.js`:

export function trackAddToCart({ id, name, price, quantity }) {
  if (typeof window !== "undefined" && window.gtag) {
    window.gtag("event", "add_to_cart", {
      item_id: id,
      item_name: name,
      price,
      quantity,
    });
  }
}

What improved:

• Analytics is a separate module—testable and replaceable
• Magic number 3000 replaced with named constant
• console.log and alert replaced with proper error state
• Cart logic is reusable across pages

---

Step 6: The Final Component

After all extractions, the ProductPage component is focused purely on composition and layout:

import { useProduct } from "../hooks/useProduct";
import { useCart } from "../hooks/useCart";
import { toggleFavorite } from "../api/productApi";
import StarRating from "../components/StarRating";
import PriceDisplay from "../components/PriceDisplay";
import OptionSelector from "../components/OptionSelector";
import ReviewCard from "../components/ReviewCard";
import ReviewForm from "../components/ReviewForm";
import { useState } from "react";

export default function ProductPage({ productId, user }) {
  const {
    product, reviews, isFavorite, setIsFavorite, addReview, loading, error,
  } = useProduct(productId, user?.id);

  const cart = useCart();

  const [selectedSize, setSelectedSize] = useState(null);
  const [selectedColor, setSelectedColor] = useState(null);
  const [quantity, setQuantity] = useState(1);

  // set defaults once product loads
  if (product && !selectedSize) setSelectedSize(product.sizes[0]);
  if (product && !selectedColor) setSelectedColor(product.colors[0]);

  if (loading) return <div className="loading">Loading...</div>;
  if (error) return <div className="error">Error: {error}</div>;
  if (!product) return <div>Product not found</div>;

  const averageRating = reviews.length > 0
    ? reviews.reduce((sum, r) => sum + r.rating, 0) / reviews.length
    : 0;

  function handleAddToCart() {
    if (!selectedSize || !selectedColor) return;
    cart.addItem(
      { productId, size: selectedSize, color: selectedColor, quantity, price: product.price },
      { id: productId, name: product.name, price: product.price, quantity },
    );
  }

  async function handleToggleFavorite() {
    try {
      await toggleFavorite(user.id, productId, isFavorite);
      setIsFavorite(!isFavorite);
    } catch { /* favorite toggle is non-critical */ }
  }

  return (
    <div className="product-page">
      <div className="product-header">
        <img src={product.image} alt={product.name} className="product-image" />
        <div className="product-info">
          <h1>{product.name}</h1>
          <StarRating rating={averageRating} count={reviews.length} />
          <PriceDisplay price={product.price} discountPercent={product.discount} />
          <p className="description">{product.description}</p>

          <OptionSelector
            label="Size" options={product.sizes}
            selected={selectedSize} onSelect={setSelectedSize}
          />
          <OptionSelector
            label="Color" options={product.colors}
            selected={selectedColor} onSelect={setSelectedColor}
          />

          <div className="actions">
            <input
              type="number" min={1} max={10}
              value={quantity}
              onChange={(e) => setQuantity(Number(e.target.value))}
            />
            <button
              onClick={handleAddToCart}
              disabled={cart.adding || !selectedSize || !selectedColor}
              className="add-to-cart"
            >
              {cart.adding ? "Adding..." : `Add to Cart (${cart.cartCount})`}
            </button>
            {user && (
              <button onClick={handleToggleFavorite} className="favorite">
                {isFavorite ? "♥ Favorited" : "♡ Favorite"}
              </button>
            )}
          </div>
          {cart.showSuccess && <div className="success-message">Added to cart!</div>}
          {cart.error && <div className="error-message">{cart.error}</div>}
        </div>
      </div>

      <section className="reviews-section">
        <h2>Reviews ({reviews.length})</h2>
        {user && (
          <ReviewForm productId={productId} user={user} onReviewAdded={addReview} />
        )}
        <div className="review-list">
          {reviews.map((review) => (
            <ReviewCard key={review.id} review={review} />
          ))}
        </div>
      </section>
    </div>
  );
}

---

Summary of Changes

Final File Structure

components/
  StarRating.jsx          — 15 lines, reusable
  OptionSelector.jsx      — 18 lines, reusable
  PriceDisplay.jsx        — 16 lines, reusable
  ReviewCard.jsx          — 16 lines, reusable
  ReviewForm.jsx          — 52 lines, self-contained
hooks/
  useProduct.js           — 48 lines, data management
  useCart.js              — 32 lines, cart operations
api/
  productApi.js           — 45 lines, API layer
analytics.js              — 10 lines, tracking
ProductPage.jsx           — 75 lines, composition only

Key Principles Applied

1. Single Responsibility — Each file has one reason to change 2. Separation of Concerns — API, state, UI, and analytics are isolated 3. DRY — Star rating, option selectors, and fetch patterns defined once 4. Error Handling — No alert() or console.log; errors shown in UI with proper state 5. Named Constants — Magic numbers replaced with descriptive identifiers 6. Composition over Complexity — The main component assembles small, testable pieces 7. Custom Hooks — State logic extracted into reusable hooks with cleanup

Code Smells Reference

A catalog of code smells—indicators in source code that suggest deeper structural problems. Each entry describes the smell, how to detect it, its severity, and recommended refactoring.

---

Bloaters

Smells where code has grown too large to work with effectively.

Long Method

Description: A method that does too much, making it hard to understand, test, and modify. Rule of thumb: if you need a comment to explain a section, that section should be its own method.

Detection Signals:

• Function body exceeds 20-30 lines
• Multiple levels of abstraction within one function
• Inline comments explaining "what this block does"
• Difficult to name the function accurately

Severity: High — affects readability, testability, and maintainability

Recommended Refactoring:

• Extract Method for logical subsections
• Decompose Conditional for complex if/else chains
• Replace Temp with Query to reduce local variables
• Introduce Parameter Object if the method has many parameters

---

Large Class

Description: A class that has taken on too many responsibilities. It typically has many fields, methods, and a name that can't describe everything it does.

Detection Signals:

• Class has 10+ methods or 7+ fields
• The class name includes "Manager", "Handler", "Processor", "Utils"
• Instance variables cluster into groups that are used independently
• Multiple developers frequently edit the same class (merge conflicts)

Severity: High — violates Single Responsibility Principle, makes changes risky

Recommended Refactoring:

• Extract Class for cohesive field/method groups
• Extract Subclass if behaviors vary by type
• Extract Interface to define role-specific contracts

---

Long Parameter List

Description: A method takes more parameters than it can comfortably handle, making calls confusing and error-prone.

Detection Signals:

• More than 3-4 parameters
• Boolean parameters (isAdmin, includeDeleted) that toggle behavior
• Parameters that always travel together
• Callers frequently pass null or default values for unused params

Severity: Medium — impacts readability and ease of calling

Recommended Refactoring:

• Introduce Parameter Object
• Preserve Whole Object (pass the object instead of pulling fields)
• Replace Parameter with Method (let the callee query what it needs)

---

Primitive Obsession

Description: Using primitive types (strings, numbers, booleans) to represent domain concepts instead of small objects or types.

Detection Signals:

• Phone numbers, emails, currencies stored as plain strings
• Status represented as string literals ("active", "pending")
• Validation logic scattered wherever the primitive is used
• Constants like "USD", "EUR" used in multiple places

Severity: Medium — leads to duplication, invalid state, and scattered validation

Recommended Refactoring:

• Replace Data Value with Object (Money, Email, PhoneNumber)
• Replace Type Code with Subclasses or Strategy
• Introduce enum/union types for finite sets of values

// Before: primitive obsession
function formatPrice(amount: number, currency: string): string { ... }

// After: value object
class Money {
  constructor(readonly amount: number, readonly currency: Currency) {}
  format(): string { ... }
}

---

Object-Orientation Abusers

Smells that indicate misuse of OO principles.

Switch Statements

Description: Complex switch/if-else chains that dispatch based on type codes or string comparisons, often duplicated across the codebase.

Detection Signals:

• Same switch on the same type code in multiple methods
• Adding a new type requires changes in many places
• Default/else branch throws or does nothing

Severity: Medium-High — violates Open/Closed Principle when types grow

Recommended Refactoring:

• Replace Conditional with Polymorphism
• Replace Type Code with Strategy pattern
• Use a lookup map for simple value dispatch

---

Feature Envy

Description: A method that uses more data or methods from another class than its own. The method "envies" another class's features.

Detection Signals:

• The method accesses 3+ fields/methods of another object
• The method barely uses its own class's data
• Chained property access: order.customer.address.city

Severity: Medium — indicates misplaced logic, poor cohesion

Recommended Refactoring:

• Move Method to the class whose data it mostly uses
• Extract Method then Move Method for partial envy
• If the method orchestrates multiple objects, it may belong in a service

---

Refused Bequest

Description: A subclass inherits methods or data it doesn't need or use, indicating a flawed hierarchy.

Detection Signals:

• Subclass overrides parent methods to throw errors or return nothing
• Subclass only uses a small fraction of inherited behavior
• The "is-a" relationship doesn't hold conceptually

Severity: Medium — creates confusing contracts and fragile hierarchies

Recommended Refactoring:

• Replace Inheritance with Delegation (composition over inheritance)
• Extract Superclass to share only the common behavior
• Push Down Method/Field to move unused members out of the parent

---

Change Preventers

Smells that make changes difficult and far-reaching.

Divergent Change

Description: A single class is modified for many different reasons. Each change touches the same class but for unrelated concerns.

Detection Signals:

• "Every time we add a new report type, we change this class"
• "Every time we change the database, we change this class"
• Multiple unrelated feature branches modify the same file

Severity: High — the class is doing too much

Recommended Refactoring:

• Extract Class to separate each axis of change
• Apply Single Responsibility Principle
• Create separate modules for separate concerns

---

Shotgun Surgery

Description: A single change requires small modifications in many different classes. The opposite of Divergent Change.

Detection Signals:

• Adding a field requires editing 5+ files
• A "simple" change creates a large diff across many modules
• Related logic is scattered without a unifying abstraction

Severity: High — high risk of missing a spot, fragile codebase

Recommended Refactoring:

• Move Method / Move Field to consolidate scattered logic
• Inline Class if the split was too granular
• Introduce a Facade or Service to centralize the concern

---

Parallel Inheritance Hierarchies

Description: Every time you create a subclass in one hierarchy, you must create a corresponding subclass in another.

Detection Signals:

• Class names mirror each other: OrderProcessor/OrderValidator, ReportPDF/ReportCSV
• Adding a type to one hierarchy always requires adding to another
• Hierarchies grow in lockstep

Severity: Medium — leads to class explosion and tight coupling

Recommended Refactoring:

• Move Method to collapse one hierarchy into the other
• Use composition to eliminate the parallel structure
• Apply Strategy or Visitor pattern

---

Dispensables

Code elements that contribute nothing and should be removed.

Dead Code

Description: Code that is never executed—unreachable branches, unused variables, commented-out blocks, unexported functions.

Detection Signals:

• IDE warnings for unused variables/imports
• git blame shows code unchanged for years
• Commented-out blocks with no explanation
• Functions with zero references/callers

Severity: Medium — clutters the codebase, confuses readers, makes coverage misleading

Recommended Refactoring:

• Delete it. Version control preserves history.
• Remove unused imports (IDE auto-fix or linter)
• Remove commented-out code blocks

---

Lazy Class

Description: A class that doesn't do enough to justify its existence. It may have been created for anticipated complexity that never materialized.

Detection Signals:

• Class has 1-2 trivial methods
• Class is a thin wrapper that delegates everything
• No unique behavior beyond what its fields provide
• Class was created "just in case"

Severity: Low-Medium — adds unnecessary indirection

Recommended Refactoring:

• Inline Class—merge it into the class that uses it
• Collapse Hierarchy if it's a subclass with no unique behavior

---

Speculative Generality

Description: Abstractions, parameters, or infrastructure added for hypothetical future needs that may never come.

Detection Signals:

• Abstract classes with only one concrete subclass
• Parameters/hooks that are never used (options, config, flags)
• "We might need this later" comments
• Generic frameworks wrapping simple operations

Severity: Medium — adds complexity without providing value (YAGNI violation)

Recommended Refactoring:

• Collapse Hierarchy for single-subclass abstractions
• Inline Class for unnecessary delegation
• Remove unused parameters and hooks
• Delete framework code that wraps trivial operations

---

Duplicated Code

Description: The same (or very similar) code structure appears in multiple places. The most common and damaging smell.

Detection Signals:

• Copy-paste patterns across methods/classes/files
• Similar logic with minor variations (different variable names, slight condition changes)
• Bug fixes applied in one copy but not others
• Linter reports duplicate blocks

Severity: High — bugs must be fixed in multiple places, risk of inconsistency

Recommended Refactoring:

• Extract Method for duplicates within a class
• Extract Superclass / Pull Up Method for duplicates across sibling classes
• Extract a shared utility module for duplicates across unrelated classes
• Template Method pattern for algorithms with varying steps

---

Couplers

Smells that create excessive coupling between classes.

Message Chains

Description: A client asks object A for object B, then asks B for object C, then asks C for D—long chains of navigation.

Detection Signals:

• a.getB().getC().getD().doSomething()
• Changes in any intermediate class break the chain
• The client knows the internal structure of multiple objects

Severity: Medium — tight coupling to object graph structure (Law of Demeter violation)

Recommended Refactoring:

• Hide Delegate—add a method on the nearest object that encapsulates the chain
• Move the logic closer to the data it accesses
• Extract Method to name the intent of the chain

---

Inappropriate Intimacy

Description: Two classes excessively access each other's internal details—private fields, internal methods, or implementation specifics.

Detection Signals:

• Classes access each other's private/protected members
• Bidirectional dependencies between classes
• Changing one class always requires changing the other
• Classes "reach into" each other frequently

Severity: Medium-High — tight coupling, difficult to change independently

Recommended Refactoring:

• Move Method / Move Field to resolve the dependency direction
• Extract Class to create a mediating abstraction
• Replace bidirectional association with unidirectional
• Hide Delegate to restore encapsulation

---

Middle Man

Description: A class that delegates almost everything to another class, adding no value itself.

Detection Signals:

• Most methods are single-line delegations to another object
• The class has no logic of its own
• Removing the class would simplify the code
• The class exists only to "wrap" another class

Severity: Low-Medium — unnecessary indirection

Recommended Refactoring:

• Remove Middle Man—let clients talk directly to the delegate
• Inline Class if the wrapper adds zero value

---

Severity Quick Reference

Refactoring Catalog

Comprehensive catalog of refactoring techniques organized by category. Each entry includes when to apply, before/after examples, and common pitfalls.

---

1. Extract Method

Category: Composing Methods

When to use:

• A code fragment can be grouped together and given a descriptive name
• A method is too long and does multiple things
• Comments explain what a block of code does (the method name should replace the comment)

Before:

function printInvoice(invoice) {
  console.log("===== Invoice =====");
  console.log(`Date: ${invoice.date}`);

  // calculate total
  let total = 0;
  for (const item of invoice.items) {
    total += item.price * item.quantity;
    if (item.taxable) {
      total += item.price * item.quantity * 0.07;
    }
  }

  // apply discount
  if (invoice.customerType === "premium") {
    total *= 0.9;
  } else if (invoice.customerType === "vip") {
    total *= 0.85;
  }

  console.log(`Total: $${total.toFixed(2)}`);
}

After:

function printInvoice(invoice) {
  printHeader(invoice);
  const total = calculateTotal(invoice);
  console.log(`Total: $${total.toFixed(2)}`);
}

function calculateTotal(invoice) {
  const subtotal = calculateSubtotal(invoice.items);
  return applyDiscount(subtotal, invoice.customerType);
}

function calculateSubtotal(items) {
  return items.reduce((sum, item) => {
    const lineTotal = item.price * item.quantity;
    const tax = item.taxable ? lineTotal * 0.07 : 0;
    return sum + lineTotal + tax;
  }, 0);
}

function applyDiscount(amount, customerType) {
  const discounts = { premium: 0.9, vip: 0.85 };
  return amount * (discounts[customerType] ?? 1);
}

function printHeader(invoice) {
  console.log("===== Invoice =====");
  console.log(`Date: ${invoice.date}`);
}

Pitfalls:

• Extracting too aggressively, creating one-line methods with no reuse value
• Losing context by naming methods too generically (doStuff, process)
• Not passing enough context—relying on shared mutable state instead of parameters

---

2. Move Method

Category: Moving Features Between Objects

When to use:

• A method uses more features of another class than the one it's defined on
• Feature Envy smell detected
• A utility method belongs closer to the data it operates on

Before:

class Order {
  constructor(customer, items) {
    this.customer = customer;
    this.items = items;
  }

  getDiscountedTotal() {
    const subtotal = this.items.reduce((s, i) => s + i.price, 0);
    if (this.customer.loyaltyPoints > 1000) return subtotal * 0.85;
    if (this.customer.loyaltyPoints > 500) return subtotal * 0.9;
    return subtotal;
  }
}

After:

class Customer {
  constructor(name, loyaltyPoints) {
    this.name = name;
    this.loyaltyPoints = loyaltyPoints;
  }

  getDiscountMultiplier() {
    if (this.loyaltyPoints > 1000) return 0.85;
    if (this.loyaltyPoints > 500) return 0.9;
    return 1;
  }
}

class Order {
  constructor(customer, items) {
    this.customer = customer;
    this.items = items;
  }

  getDiscountedTotal() {
    const subtotal = this.items.reduce((s, i) => s + i.price, 0);
    return subtotal * this.customer.getDiscountMultiplier();
  }
}

Pitfalls:

• Moving a method that legitimately orchestrates multiple objects (it may belong in a service)
• Breaking the public API of a widely-used class without a migration path
• Moving to a class that already has too many responsibilities

---

3. Replace Conditional with Polymorphism

Category: Simplifying Conditional Expressions

When to use:

• A switch/if-else chain selects behavior based on a type or category
• The same conditional structure appears in multiple places
• New types are frequently added, requiring changes in many switch blocks

Before:

type Shape = { kind: "circle"; radius: number }
  | { kind: "rectangle"; width: number; height: number }
  | { kind: "triangle"; base: number; height: number };

function area(shape: Shape): number {
  switch (shape.kind) {
    case "circle":
      return Math.PI * shape.radius ** 2;
    case "rectangle":
      return shape.width * shape.height;
    case "triangle":
      return 0.5 * shape.base * shape.height;
    default:
      throw new Error(`Unknown shape: ${(shape as any).kind}`);
  }
}

function perimeter(shape: Shape): number {
  switch (shape.kind) {
    case "circle":
      return 2 * Math.PI * shape.radius;
    case "rectangle":
      return 2 * (shape.width + shape.height);
    case "triangle":
      // simplified: assumes equilateral
      return 3 * shape.base;
    default:
      throw new Error(`Unknown shape`);
  }
}

After:

interface Shape {
  area(): number;
  perimeter(): number;
}

class Circle implements Shape {
  constructor(private radius: number) {}
  area() { return Math.PI * this.radius ** 2; }
  perimeter() { return 2 * Math.PI * this.radius; }
}

class Rectangle implements Shape {
  constructor(private width: number, private height: number) {}
  area() { return this.width * this.height; }
  perimeter() { return 2 * (this.width + this.height); }
}

class Triangle implements Shape {
  constructor(private base: number, private height: number) {}
  area() { return 0.5 * this.base * this.height; }
  perimeter() { return 3 * this.base; }
}

Pitfalls:

• Over-engineering when the conditional is simple and unlikely to grow
• TypeScript discriminated unions with exhaustive checks may be preferable to class hierarchies
• Adding polymorphism when only one method varies—overkill for a single dispatch point

---

4. Introduce Parameter Object

Category: Simplifying Method Calls

When to use:

• Multiple parameters naturally group together and travel as a pack
• The same parameter group appears in several functions
• The parameter list is growing beyond 3-4 arguments

Before:

function searchProducts(
  query: string,
  minPrice: number,
  maxPrice: number,
  category: string,
  sortBy: string,
  sortOrder: "asc" | "desc",
  page: number,
  pageSize: number
) {
  // ...
}

function countProducts(
  query: string,
  minPrice: number,
  maxPrice: number,
  category: string
) {
  // ...
}

After:

interface ProductFilter {
  query: string;
  minPrice: number;
  maxPrice: number;
  category: string;
}

interface PaginationOptions {
  sortBy: string;
  sortOrder: "asc" | "desc";
  page: number;
  pageSize: number;
}

function searchProducts(filter: ProductFilter, pagination: PaginationOptions) {
  // ...
}

function countProducts(filter: ProductFilter) {
  // ...
}

Pitfalls:

• Creating a "god object" that bundles unrelated parameters
• Making optional parameters required because they're in the object
• Losing discoverability—callers now need to know the object shape

---

5. Replace Temp with Query

Category: Composing Methods

When to use:

• A temporary variable holds the result of an expression that could be a method
• The temp is used in multiple places within the method
• Extracting a query method would clarify intent

Before:

function getPrice(order) {
  const basePrice = order.quantity * order.itemPrice;
  const discount = Math.max(0, order.quantity - 100) * order.itemPrice * 0.05;
  const shipping = Math.min(basePrice * 0.1, 50);
  return basePrice - discount + shipping;
}

After:

function getPrice(order) {
  return basePrice(order) - discount(order) + shipping(order);
}

function basePrice(order) {
  return order.quantity * order.itemPrice;
}

function discount(order) {
  return Math.max(0, order.quantity - 100) * order.itemPrice * 0.05;
}

function shipping(order) {
  return Math.min(basePrice(order) * 0.1, 50);
}

Pitfalls:

• Performance cost if the query is expensive and called multiple times (cache or memoize)
• Do not apply when the temp captures a snapshot that must not change mid-method
• Over-extracting trivially simple expressions

---

6. Decompose Conditional

Category: Simplifying Conditional Expressions

When to use:

• Complex conditional logic makes the code hard to read
• The condition, then-branch, or else-branch contain substantial logic
• The reader needs to mentally parse what each branch means

Before:

function calculateCharge(date, quantity, plan) {
  let charge;
  if (
    date.getMonth() >= 5 && date.getMonth() <= 8 &&
    plan.type !== "unlimited" &&
    quantity > plan.includedUnits
  ) {
    charge = quantity * plan.summerRate + plan.summerServiceCharge;
  } else {
    charge = quantity * plan.regularRate;
  }
  return charge;
}

After:

function calculateCharge(date, quantity, plan) {
  if (isSummerSurcharge(date, quantity, plan)) {
    return summerCharge(quantity, plan);
  }
  return regularCharge(quantity, plan);
}

function isSummerSurcharge(date, quantity, plan) {
  const month = date.getMonth();
  return month >= 5 && month <= 8
    && plan.type !== "unlimited"
    && quantity > plan.includedUnits;
}

function summerCharge(quantity, plan) {
  return quantity * plan.summerRate + plan.summerServiceCharge;
}

function regularCharge(quantity, plan) {
  return quantity * plan.regularRate;
}

Pitfalls:

• Don't extract conditions that are already clear (e.g., if (user.isAdmin))
• Naming the extracted predicate poorly can make things worse, not better
• Don't scatter related logic across too many tiny functions if it harms locality

---

7. Consolidate Duplicate Conditional Fragments

Category: Simplifying Conditional Expressions

When to use:

• The same code appears in every branch of a conditional
• Code before or after a conditional is duplicated across branches

Before:

function calculateTotal(isSpecialDeal, price, quantity) {
  let total;
  if (isSpecialDeal) {
    total = price * quantity * 0.85;
    sendAnalyticsEvent("purchase", total);
    updateInventory(quantity);
  } else {
    total = price * quantity;
    sendAnalyticsEvent("purchase", total);
    updateInventory(quantity);
  }
  return total;
}

After:

function calculateTotal(isSpecialDeal, price, quantity) {
  const total = isSpecialDeal
    ? price * quantity * 0.85
    : price * quantity;

  sendAnalyticsEvent("purchase", total);
  updateInventory(quantity);
  return total;
}

Pitfalls:

• Consolidating code that only looks the same but has semantic differences
• Moving code outside the conditional when order of execution matters

---

8. Replace Magic Number with Named Constant

Category: Organizing Data

When to use:

• A numeric literal appears in code with no clear meaning
• The same value is used in multiple places
• The value could change in the future (rates, limits, thresholds)

Before:

function calculateShipping(weight, distance) {
  if (weight > 25) {
    return distance * 0.15 + 12.5;
  }
  if (distance > 500) {
    return weight * 0.08 + 7.99;
  }
  return 4.99;
}

After:

const MAX_STANDARD_WEIGHT_KG = 25;
const LONG_DISTANCE_THRESHOLD_KM = 500;
const HEAVY_RATE_PER_KM = 0.15;
const HEAVY_SURCHARGE = 12.5;
const DISTANCE_RATE_PER_KG = 0.08;
const LONG_DISTANCE_BASE = 7.99;
const STANDARD_SHIPPING = 4.99;

function calculateShipping(weight, distance) {
  if (weight > MAX_STANDARD_WEIGHT_KG) {
    return distance * HEAVY_RATE_PER_KM + HEAVY_SURCHARGE;
  }
  if (distance > LONG_DISTANCE_THRESHOLD_KM) {
    return weight * DISTANCE_RATE_PER_KG + LONG_DISTANCE_BASE;
  }
  return STANDARD_SHIPPING;
}

Pitfalls:

• Naming constants too generically (THRESHOLD, VALUE_1)
• Extracting universally obvious values (0, 1, "", 100 for percentages)
• Scattering constants far from their usage when they're only used once

---

9. Encapsulate Field

Category: Organizing Data

When to use:

• A public field is accessed directly from outside the class
• You need to add validation, transformation, or observation on field access
• You want to maintain the ability to change internal representation

Before:

class Employee {
  name: string;
  salary: number;
  department: string;

  constructor(name: string, salary: number, department: string) {
    this.name = name;
    this.salary = salary;
    this.department = department;
  }
}

// usage
employee.salary = -5000; // no validation

After:

class Employee {
  private _name: string;
  private _salary: number;
  private _department: string;

  constructor(name: string, salary: number, department: string) {
    this._name = name;
    this._salary = salary;
    this._department = department;
  }

  get name() { return this._name; }
  set name(value: string) {
    if (!value.trim()) throw new Error("Name cannot be empty");
    this._name = value.trim();
  }

  get salary() { return this._salary; }
  set salary(value: number) {
    if (value < 0) throw new Error("Salary cannot be negative");
    this._salary = value;
  }

  get department() { return this._department; }
  set department(value: string) { this._department = value; }
}

Pitfalls:

• Adding getters/setters for every field mechanically—only encapsulate when there's a reason
• In TypeScript, consider readonly for immutable fields instead of getters
• Setters that silently coerce values can hide bugs

---

10. Extract Class

Category: Moving Features Between Objects

When to use:

• A class has too many responsibilities (violates Single Responsibility Principle)
• A subset of fields and methods form a cohesive group
• The class has grown to the point where its name can't accurately describe everything it does

Before:

class User {
  name: string;
  email: string;
  street: string;
  city: string;
  state: string;
  zip: string;
  phone: string;
  phoneType: "mobile" | "home" | "work";

  getFullAddress() {
    return `${this.street}\n${this.city}, ${this.state} ${this.zip}`;
  }

  getFormattedPhone() {
    return `(${this.phone.slice(0, 3)}) ${this.phone.slice(3, 6)}-${this.phone.slice(6)}`;
  }

  validateAddress() {
    return this.street && this.city && this.state && this.zip?.length === 5;
  }
}

After:

class Address {
  constructor(
    public street: string,
    public city: string,
    public state: string,
    public zip: string
  ) {}

  format() {
    return `${this.street}\n${this.city}, ${this.state} ${this.zip}`;
  }

  isValid() {
    return !!(this.street && this.city && this.state && this.zip?.length === 5);
  }
}

class Phone {
  constructor(
    public number: string,
    public type: "mobile" | "home" | "work"
  ) {}

  format() {
    return `(${this.number.slice(0, 3)}) ${this.number.slice(3, 6)}-${this.number.slice(6)}`;
  }
}

class User {
  constructor(
    public name: string,
    public email: string,
    public address: Address,
    public phone: Phone
  ) {}
}

Pitfalls:

• Extracting too early before the class has actually grown
• Creating classes with only data and no behavior (anemic domain model)
• Breaking existing consumers that depend on the flat structure

---

11. Replace Error Code with Exception

Category: Making Method Calls Simpler

When to use:

• A method returns a special value (like -1, null, or false) to indicate an error
• Callers forget to check the return value, leading to silent failures
• Error handling logic pollutes the main flow

Before:

function withdraw(account, amount) {
  if (amount <= 0) return -1;
  if (amount > account.balance) return -2;
  account.balance -= amount;
  return account.balance;
}

// caller must remember to check
const result = withdraw(account, 500);
if (result === -1) { /* invalid amount */ }
else if (result === -2) { /* insufficient funds */ }

After:

class InvalidAmountError extends Error {
  constructor(amount) {
    super(`Invalid withdrawal amount: ${amount}`);
    this.name = "InvalidAmountError";
  }
}

class InsufficientFundsError extends Error {
  constructor(balance, amount) {
    super(`Cannot withdraw ${amount} from balance of ${balance}`);
    this.name = "InsufficientFundsError";
  }
}

function withdraw(account, amount) {
  if (amount <= 0) throw new InvalidAmountError(amount);
  if (amount > account.balance) throw new InsufficientFundsError(account.balance, amount);
  account.balance -= amount;
  return account.balance;
}

Pitfalls:

• Throwing exceptions for expected flow control (e.g., "user not found" in a search)
• Not creating specific error classes—generic Error loses context
• Missing error boundaries in async code (unhandled promise rejections)

---

12. Inline Method / Inline Temp

Category: Composing Methods

When to use:

• A method body is as clear as its name
• A temp variable is assigned once and used immediately
• An extracted method adds indirection without adding clarity

Before:

function getRating(driver) {
  return moreThanFiveLateDeliveries(driver) ? 2 : 1;
}

function moreThanFiveLateDeliveries(driver) {
  return driver.numberOfLateDeliveries > 5;
}

After:

function getRating(driver) {
  return driver.numberOfLateDeliveries > 5 ? 2 : 1;
}

Pitfalls:

• Inlining a method that is used in multiple places—creates duplication
• Inlining when the extracted name genuinely improves readability
• Inlining complex expressions that become hard to read on one line

---

Quick Reference Table