Code Quality Skill

[!FIXED!]

Purpose

This skill provides procedures for maintaining high code quality standards. It covers code review preparation, refactoring guidelines, and quality checks.

When to use:

• Before submitting code for review
• During refactoring
• When reviewing others' code
• When quality issues are identified [!FIXED!]

=== PREREQUISITES ===

Before using this skill:

• Access to @ref(CB-STD-PATTERNS-001) if exists
• Access to @ref(CB-STD-NAMING-001) if exists
• Understanding of project's linter/formatter settings
• Test framework available

=== PROCEDURE: PRE-REVIEW CHECKLIST ===

Run this checklist before submitting code for review.

Automated Checks

1. Run linter

   bashCopy

   # Example for JavaScript
   npm run lint
   
   # Fix auto-fixable issues
   npm run lint:fix
   

2. Run formatter

   bashCopy

   # Example for Prettier
   npm run format
   

3. Run tests

   bashCopy

   npm test
   

4. Check coverage

   bashCopy

   npm run test:coverage
   

Manual Checks

• No commented-out code
• No console.log/print debug statements
• No hardcoded values (use constants)
• No magic numbers
• All functions have purpose comments (if complex)
• Variable names are descriptive
• Error handling is appropriate
• No duplicate code (DRY)
• Functions are single-purpose
• File length is reasonable (<300 lines preferred)

Self-Review Questions

• Does this code do what the specification requires?
• Would I understand this code in 6 months?
• Are edge cases handled?
• Are error messages helpful?
• Is this the simplest solution?

=== PROCEDURE: REFACTORING ===

When to Refactor

Refactoring Steps

1. Write tests first

   • Ensure existing behavior is covered
   • Tests should pass before AND after refactoring

2. Make small changes

   • One refactoring at a time
   • Commit between refactorings
   • Keep changes reversible

3. Verify tests still pass

   • After each change
   • No behavior change unless intentional

4. Document in buildlog

   markdownCopy

   | HH:MM | #micro-decision | Refactored payment processing to use strategy pattern | CB-ARCH-OVERVIEW |
   

Common Refactorings

=== PROCEDURE: NAMING ===

General Rules

• Names should reveal intent
• Use pronounceable names
• Use searchable names
• Avoid encodings (Hungarian notation)

Naming Patterns

Name Length Guidelines

=== PROCEDURE: ERROR HANDLING ===

Error Handling Guidelines

1. Fail fast

   • Validate inputs early
   • Return early on error conditions

2. Be specific

   • Use specific error types
   • Include context in error messages

3. Don't swallow errors

   javascriptCopy

   // BAD
   try { doSomething() } catch (e) { }
   
   // GOOD
   try { doSomething() } catch (e) {
     logger.error('Failed to do something', { error: e });
     throw e;
   }
   

4. Handle at appropriate level

   • Don't catch if you can't handle
   • Let errors bubble up when appropriate

Error Message Format

[What happened]: [Why it matters]: [What to do]

Example:
"Failed to load user profile: User ID not found: Please verify the user exists"

=== PROCEDURE: DOCUMENTATION ===

When to Document

• Complex algorithms
• Non-obvious business logic
• Public APIs
• Configuration options
• Workarounds for known issues

When NOT to Document

• Obvious code
• Self-explanatory functions
• Standard patterns

Comment Format

/**
 * Calculates the discounted price based on user tier.
 *
 * Premium users get 20% off, Gold users get 10% off.
 * Discounts don't stack with sale prices.
 *
 * @param {number} price - Original price in cents
 * @param {string} userTier - User membership tier
 * @returns {number} Final price in cents
 */
function calculateDiscountedPrice(price, userTier) { ... }

Inline Comments

// Use local timezone for display but UTC for storage
const displayDate = formatLocal(date);
const storageDate = date.toISOString();

=== PROCEDURE: TESTING ===

Test Structure

Arrange: Set up test data and conditions
Act: Execute the code under test
Assert: Verify the results

Test Naming

describe('calculateDiscount', () => {
  it('should apply 20% discount for premium users', () => { ... });
  it('should return original price for basic users', () => { ... });
  it('should throw error for negative prices', () => { ... });
});

Coverage Guidelines

What to Test

• Happy path (expected inputs)
• Edge cases (boundaries, empty, null)
• Error conditions
• Security-sensitive operations

=== ANTI-PATTERNS ===

=== QUALITY METRICS ===

=== RELATED DOCUMENTS ===