claude-code-ultimate-guide/examples/skills/design-patterns/checklists/pattern-evaluation.md

title	description	tags
Design Pattern Quality Evaluation Checklist	Systematic scoring criteria for evaluating design pattern implementation quality	cheatsheet design-patterns code-review

Design Pattern Quality Evaluation Checklist

Systematic criteria for evaluating the quality of design pattern implementations.

Evaluation Criteria

Each criterion is scored 0-10, where:

• 9-10: Exemplary, reference-quality implementation
• 7-8: Good, minor improvements possible
• 5-6: Acceptable, notable issues to address
• 3-4: Problematic, significant refactoring needed
• 0-2: Incorrect or severely flawed

Overall Score = Average of all criteria scores

---

1. Correctness (0-10)

Question: Does the implementation correctly follow the canonical pattern structure?

Scoring Guidelines

Score	Description
9-10	Perfect adherence to pattern structure, all roles present and correctly implemented
7-8	Minor deviations that don't compromise pattern intent
5-6	Some structural issues but pattern is recognizable and functional
3-4	Significant structural problems, pattern only partially implemented
0-2	Incorrect implementation, doesn't match pattern at all

Checklist

Singleton:

• Private constructor
• Static getInstance() method
• Private static instance field
• Thread-safe (if relevant)
• Returns same instance every time

Observer:

• Subject interface with attach/detach/notify
• Observer interface with update method
• Subject maintains list of observers
• Notify calls update on all observers
• Observers can be added/removed dynamically

Strategy:

• Strategy interface defining algorithm
• Context holds strategy reference
• Context delegates to strategy
• Strategies are interchangeable
• Client can set strategy at runtime

Factory Method:

• Factory method returns interface/abstract class
• Subclasses override factory method
• Client code depends on interface, not concrete classes
• Creation logic is encapsulated

Decorator:

• Decorator implements same interface as wrapped object
• Decorator holds reference to wrapped object
• Decorator delegates to wrapped object
• Can stack multiple decorators
• Maintains interface contract

Common Mistakes (Deductions)

• -2: Missing key component (e.g., Singleton without private constructor)
• -3: Incorrect delegation (e.g., Decorator not calling wrapped object)
• -4: Breaking pattern invariants (e.g., Singleton returning different instances)

---

2. Testability (0-10)

Question: How easy is it to write unit tests for this implementation?

Scoring Guidelines

Score	Description
9-10	Easily mockable, injectable dependencies, no global state
7-8	Testable with minor setup, some coupling exists
5-6	Requires significant test setup, moderate coupling
3-4	Hard to test, tight coupling, global state
0-2	Nearly impossible to test, static dependencies, no injection points

Checklist

• Dependencies are injected (not created internally)
• Interfaces are used (can be mocked)
• No hard-coded dependencies
• No global state access (or minimal)
• Test isolation is possible (tests don't affect each other)
• No static methods that can't be mocked
• Side effects are minimized or controllable

Red Flags (Deductions)

• -2: Using getInstance() instead of dependency injection
• -2: Hard-coded concrete class instantiation
• -3: Accessing global state (window, global, process.env in business logic)
• -3: Static methods with side effects
• -4: No way to inject test doubles

Examples

Bad Testability (Score: 2/10):

class PaymentService {
  processPayment(amount: number) {
    // Hard to test: creates dependency internally
    const gateway = PaymentGateway.getInstance();
    // Hard to test: accesses global config
    const apiKey = process.env.PAYMENT_API_KEY;
    return gateway.charge(amount, apiKey);
  }
}

Good Testability (Score: 9/10):

class PaymentService {
  constructor(
    private gateway: IPaymentGateway,
    private config: Config
  ) {}

  processPayment(amount: number) {
    const apiKey = this.config.getPaymentApiKey();
    return this.gateway.charge(amount, apiKey);
  }
}

// Easy to test with mocks
const mockGateway = { charge: jest.fn() };
const mockConfig = { getPaymentApiKey: () => 'test-key' };
const service = new PaymentService(mockGateway, mockConfig);

---

3. Single Responsibility Principle (0-10)

Question: Does the component have one, clearly defined responsibility?

Scoring Guidelines

Score	Description
9-10	Single, focused responsibility; class has one reason to change
7-8	Mostly focused, minor secondary concerns
5-6	Multiple related responsibilities
3-4	Several unrelated responsibilities
0-2	God class with many responsibilities

Checklist

• Class/module has one clear purpose
• All methods relate to the primary responsibility
• Changing one requirement doesn't necessitate changing this class
• Class name clearly reflects its responsibility
• No "and" in class name or description (e.g., "UserManagerAndLogger" is bad)

Red Flags (Deductions)

• -2: Class handles 2 distinct concerns
• -3: Class handles 3+ concerns
• -4: God class (>20 methods, >300 lines)
• -1: Methods unrelated to primary responsibility

Examples

Poor SRP (Score: 3/10):

class UserService {
  createUser(data: UserData) { /* ... */ }
  validateEmail(email: string) { /* ... */ }
  sendWelcomeEmail(user: User) { /* ... */ }
  logUserActivity(activity: string) { /* ... */ }
  generateReport(userId: string) { /* ... */ }
  // Too many responsibilities: creation, validation, email, logging, reporting
}

Good SRP (Score: 9/10):

class UserService {
  constructor(
    private validator: UserValidator,
    private emailService: EmailService,
    private logger: Logger
  ) {}

  createUser(data: UserData): User {
    // Focuses only on user creation orchestration
    this.validator.validate(data);
    const user = new User(data);
    this.emailService.sendWelcomeEmail(user);
    this.logger.logActivity('user_created', user.id);
    return user;
  }
}

---

4. Open/Closed Principle (0-10)

Question: Can the component be extended without modifying its source code?

Scoring Guidelines

Score	Description
9-10	Fully extensible via inheritance or composition, no modification needed
7-8	Mostly extensible, minor modifications might be needed
5-6	Some extension points exist but limited
3-4	Hard to extend, requires modification in multiple places
0-2	Closed for extension, must modify source code

Checklist

• Uses interfaces or abstract classes
• New behavior can be added via new classes, not modifications
• Configuration or strategy pattern for varying behavior
• No switch statements on types (if adding new type requires modification)
• Dependency inversion (depends on abstractions)

Red Flags (Deductions)

• -2: Switch on type (adding new type requires modification)
• -3: No interfaces (concrete dependencies everywhere)
• -3: Hard-coded behavior (no extension points)
• -4: Modifying existing methods is the only way to add features

Examples

Closed for Extension (Score: 2/10):

class PaymentProcessor {
  process(type: string, amount: number) {
    switch (type) {
      case 'credit': return this.processCreditCard(amount);
      case 'paypal': return this.processPaypal(amount);
      // Adding crypto payment requires modifying this class
    }
  }
}

Open for Extension (Score: 9/10):

interface PaymentStrategy {
  process(amount: number): Promise<Receipt>;
}

class PaymentProcessor {
  constructor(private strategies: Map<string, PaymentStrategy>) {}

  process(type: string, amount: number) {
    const strategy = this.strategies.get(type);
    if (!strategy) throw new Error(`Unknown payment type: ${type}`);
    return strategy.process(amount);
  }
}

// Add new payment method without modifying PaymentProcessor
class CryptoPaymentStrategy implements PaymentStrategy {
  process(amount: number) { /* ... */ }
}

---

5. Documentation (0-10)

Question: Is the implementation well-documented with clear intent and usage?

Scoring Guidelines

Score	Description
9-10	Comprehensive documentation: intent, usage, examples, edge cases
7-8	Good documentation, covers main use cases
5-6	Basic documentation, minimal but present
3-4	Sparse documentation, unclear intent
0-2	No documentation or misleading documentation

Checklist

• Class/interface has JSDoc/TSDoc comment explaining purpose
• Pattern intent is documented ("This is a Singleton because...")
• Public methods have documentation
• Complex logic has inline comments
• Usage examples are provided (in README or comments)
• Invariants and constraints are documented
• Naming is self-documenting (clear, descriptive names)

Red Flags (Deductions)

• -2: No class-level documentation
• -2: Public API methods undocumented
• -3: Cryptic naming (x, foo, temp, data)
• -1: Complex logic without explanation

Examples

Poor Documentation (Score: 2/10):

class S {
  private static i: S;
  private constructor() {}
  static get() {
    if (!S.i) S.i = new S();
    return S.i;
  }
  do(x: any) { /* ... */ }
}

Good Documentation (Score: 9/10):

/**
 * Configuration service implemented as a Singleton to ensure
 * all components share the same configuration state.
 *
 * Use `ConfigService.getInstance()` to access the singleton instance.
 *
 * @example
 * const config = ConfigService.getInstance();
 * const apiUrl = config.get('API_URL');
 */
class ConfigService {
  private static instance: ConfigService;

  /**
   * Private constructor prevents direct instantiation.
   * Use `getInstance()` instead.
   */
  private constructor() {
    // Load configuration from environment
  }

  /**
   * Returns the singleton instance of ConfigService.
   * Creates the instance on first call (lazy initialization).
   *
   * @returns The singleton ConfigService instance
   */
  public static getInstance(): ConfigService {
    if (!ConfigService.instance) {
      ConfigService.instance = new ConfigService();
    }
    return ConfigService.instance;
  }

  /**
   * Retrieves a configuration value by key.
   *
   * @param key - Configuration key
   * @returns Configuration value or undefined if not found
   */
  public get(key: string): string | undefined {
    return process.env[key];
  }
}

---

Pattern-Specific Evaluation

Singleton Specific

Additional Checklist:

• Lazy initialization (if appropriate)
• Thread-safety considered (less critical in JS)
• Subclassing is prevented or controlled
• No public constructor
• Reset mechanism for tests (or DI alternative mentioned)

Deductions:

• -3: Public constructor (defeats purpose)
• -2: Multiple getInstance() methods returning different instances
• -2: No consideration of test isolation

Observer Specific

Additional Checklist:

• Observers can unsubscribe
• No memory leaks (observers are properly removed)
• Notification order is deterministic (if it matters)
• Observers don't depend on notification order
• Subject doesn't know concrete observer types

Deductions:

• -3: No unsubscribe mechanism (memory leak risk)
• -2: Subject depends on concrete observer types
• -2: Notification order matters but isn't guaranteed

Strategy Specific

Additional Checklist:

• Strategies implement common interface
• Context doesn't depend on concrete strategies
• Strategies are interchangeable
• Strategy can be set at runtime
• Strategies don't share state (unless explicitly designed to)

Deductions:

• -3: Context depends on concrete strategies
• -2: Strategies are not truly interchangeable
• -2: No way to change strategy at runtime

---

Overall Assessment Formula

Overall Score = (
  Correctness × 0.30 +
  Testability × 0.25 +
  Single Responsibility × 0.20 +
  Open/Closed × 0.15 +
  Documentation × 0.10
) / 5

Weighted because correctness is most important, followed by testability.

---

Interpretation Guide

Overall Score	Interpretation	Action
9.0 - 10.0	Excellent	Reference-quality, minimal changes needed
7.0 - 8.9	Good	Minor improvements, production-ready
5.0 - 6.9	Acceptable	Notable issues, refactoring recommended
3.0 - 4.9	Poor	Significant problems, refactoring required
0.0 - 2.9	Critical	Fundamentally flawed, redesign needed

---

Example Evaluation Report

Pattern: Singleton

File: src/services/config-singleton.ts Lines: 5-34

Scores

Criterion	Score	Rationale
Correctness	8/10	Implements singleton correctly, minor: no thread-safety (not critical in JS)
Testability	4/10	Hard to mock, no reset mechanism, global state makes tests interdependent
Single Responsibility	9/10	Focuses solely on configuration management
Open/Closed	7/10	Can add new config keys, but config sources are hard-coded
Documentation	6/10	Has JSDoc but missing rationale for singleton choice

Overall Score: 6.8/10 (Acceptable)

Issues Identified

1. High Priority: Add resetInstance() method for test isolation

   • Current: Tests must run in specific order
   • Fix: Add public static resetInstance() guarded by environment check

2. Medium Priority: Document singleton rationale

   • Current: Unclear why global state is necessary
   • Fix: Add JSDoc explaining choice (e.g., "Singleton ensures consistent config across all services")

3. Low Priority: Consider dependency injection alternative

   • Current: Hard to test, tight coupling
   • Recommendation: Evaluate using DI container or React Context

Recommendations

// Add test-friendly reset
public static resetInstance(): void {
  if (process.env.NODE_ENV === 'test') {
    ConfigService.instance = null!;
  }
}

// Better yet: refactor to DI
class ConfigService {
  constructor(private envVars: EnvVars) {}
}

// Inject in DI container or provider
const config = new ConfigService(process.env);

---

Usage in Skill

When the design-patterns skill runs in Evaluation Mode, it:

1. Identifies which pattern is implemented
2. Applies relevant checklist items
3. Scores each criterion (0-10)
4. Calculates weighted overall score
5. Generates detailed report with:
   • Scores table
   • Issues found (priority-ranked)
   • Specific recommendations with code examples
   • Stack-native alternatives if applicable

---

References

• Clean Code by Robert C. Martin (SOLID principles)
• Refactoring: Improving the Design of Existing Code by Martin Fowler
• Design Patterns: Elements of Reusable Object-Oriented Software (Gang of Four)