Files
skills/skills/refactoring/SKILL.md
T
mathiasandClaude Opus 5 56a947b2a0 refactor(plugin)!: ship as a Claude Code plugin, one dir link per harness
Skills move to skills/<name>/SKILL.md and the repo gains both manifests, so
it is now a marketplace hosting exactly one plugin whose root is the repo
root (source: "."). Claude Code installs it from git; no symlinks involved.

Why: install.sh wired the library into every consuming repo as an ABSOLUTE
symlink to $CHECKOUT_DIR. The checkout moved three times, most recently when
local-dev@1966d5a dropped the embedded .skills/ copy, and nothing migrated
the links — 243 dangling links across 12 repos, hidden from git by the
**/.claude/skills/ ignore rule. Worse, ~/dev spans two path roots
(/Users/mathias on the iCloud-shared Macs, /home/mathias on koala), so an
absolute link stored there is broken on one side by construction. Re-running
the installer cannot fix that; only removing the mechanism can.

install.sh keeps the harnesses with no plugin system and drops from 42 links
per harness to one directory link, via the convergent ~/.agents/skills
convention that Pi and OpenCode both read:

  ~/.agents/skills          -> $CHECKOUT_DIR/skills
  ~/.config/crush/skills    -> ~/.agents/skills
  ~/.config/opencode/skills -> ~/.agents/skills

wire_claude_repo is deleted outright rather than gated behind a flag — on a
mixed-root fleet it cannot be made correct. Antigravity and Vibe targets go
too; both unused. CHECKOUT_DIR keeps its host-local default deliberately:
pointing it into ~/dev would put a git working tree inside iCloud with two
concurrent writers.

Taskfile drops five near-identical wiring loops (they had already drifted)
and delegates to install.sh. `task check` gains frontmatter validation, a
stray-root-entry gate, and manifest/tag version parity — the loose
secret-hygiene.md sat unloadable for months because nothing checked layout.

Verified: claude plugin validate . passes clean; a probe skill loaded as
skills:zzprobe through --plugin-dir; task check green at 42 skills.

Refs: #23
Bump-Type: minor

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BeAp5LTscnz2W7Ubt4eJ6m
2026-08-23 14:29:51 +02:00

9.4 KiB

name, description
name description
refactoring Systematic refactoring methodology based on Martin Fowler's catalog. Apply during the REFACTOR phase of TDD, never while making code changes that modify behavior.

Refactoring

Core Principle

"Refactoring is a controlled technique for improving the design of existing code. Its essence is applying a series of small behavior-preserving transformations." — Martin Fowler

The First Rule: Never refactor without tests. Tests are your safety net. If tests don't exist, write them first (load tdd skill).

The Second Rule: Each refactoring step must keep tests green. If a step breaks tests, undo it.

Never mix refactoring and behavior change. Separate commits for each.

When to Refactor

  • After the GREEN phase of TDD (REFACTOR phase)
  • When code smell makes a feature hard to add
  • When the Boy Scout Rule demands it — you're in the area, leave it better
  • When technical debt is blocking velocity

When NOT to refactor:

  • Code that works and won't change
  • Code being replaced soon
  • When you don't have tests

The Process

Step 1: Ensure Test Coverage

go test ./...          # All tests pass first
go test -race ./...    # Race detector clean

If tests don't exist, write them before refactoring. See the tdd skill.

Step 2: Identify the Target

Load the code-review skill to detect smells. Prioritize:

  1. Architectural smells — Shotgun Surgery, Divergent Change, God Objects
  2. Design smells — Long Methods, Feature Envy, Primitive Obsession
  3. Readability smells — naming, comments, duplication

Step 3: Apply Techniques in Small Steps

Each technique below is one small step. Make the change. Run tests. Commit if green. Repeat.

Step 4: Verify After Each Step

go test ./...
go test -race ./...

If red: undo the step (git checkout), understand why, try a smaller step.

Technique Catalog

Composing Methods

Extract Function When: a code block can be named meaningfully; a function does multiple things.

// Before
func processOrder(o *Order) {
    // validate
    if o.Items == nil { panic("no items") }
    // calculate
    total := 0.0
    for _, item := range o.Items {
        total += item.Price
    }
    // save
    db.Save(o, total)
}

// After: each step extracted
func processOrder(o *Order) {
    validateOrder(o)
    total := calculateTotal(o.Items)
    saveOrder(o, total)
}

Cross-reference: when naming extracted functions, load clean-code skill for naming conventions.

Inline Function When: a function's body is as clear as its name; a function only does what its name says.

Extract Variable When: a complex expression is hard to read inline.

// Before
if user.Subscription.Level >= 2 && !user.IsBanned && user.VerifiedAt != nil { ... }

// After
canAccessPremium := user.Subscription.Level >= 2 && !user.IsBanned && user.VerifiedAt != nil
if canAccessPremium { ... }

Replace Temp with Query When: a temporary variable holds the result of an expression used once.

Moving Features Between Types

Move Method When: Feature Envy detected — a method uses another type's data more than its own.

// Before: Order envies Customer
func (o *Order) getShippingCost() float64 {
    if o.Customer.Country == "SE" { return 0 }
    return 50
}

// After: moved to Customer
func (c *Customer) ShippingCost() float64 {
    if c.Country == "SE" { return 0 }
    return 50
}

Extract Type When: a cluster of methods and fields have a different responsibility than the rest of the type.

Inline Type When: a type isn't doing enough to justify its existence (Lazy Class smell).

Hide Delegate When: callers access objects through chains (a.B().C()).

Remove Middle Man When: a type only delegates without adding value.

Organizing Data

Replace Data Value with Object When: Primitive Obsession detected — a string is being used for email, user ID, currency.

// Before
type User struct {
    Email string
}

// After: Email type carries validation and prevents misuse
type Email struct {
    value string
}

func NewEmail(s string) (Email, error) {
    if !strings.Contains(s, "@") {
        return Email{}, fmt.Errorf("invalid email: %q", s)
    }
    return Email{value: s}, nil
}

type User struct {
    Email Email
}

Replace Magic Number with Constant

// Before
if retries > 3 { ... }

// After
const maxRetries = 3
if retries > maxRetries { ... }

Simplifying Conditionals

Decompose Conditional When: complex conditionals obscure intent.

// Before
if plan.ExpiresAt.Before(time.Now()) && plan.GracePeriodDays > 0 { ... }

// After
if plan.IsExpiredWithGrace() { ... }

Replace Nested Conditional with Guard Clauses When: deep nesting; use early returns instead.

// Before: arrow code
func process(o *Order) error {
    if o != nil {
        if len(o.Items) > 0 {
            if o.Customer != nil {
                // actual logic
            }
        }
    }
    return nil
}

// After: guard clauses
func process(o *Order) error {
    if o == nil {
        return errors.New("nil order")
    }
    if len(o.Items) == 0 {
        return errors.New("empty order")
    }
    if o.Customer == nil {
        return errors.New("no customer")
    }
    // actual logic
    return nil
}

Replace Conditional with Polymorphism When: repeated switch/if-else on a type string. Replace with an interface. See the solid skill, OCP section for full example.

Introduce Null Object When: repeated nil checks for a dependency.

Simplifying Function Calls

Rename Method/Variable The most powerful refactoring. When you understand what something does, give it a name that reflects that understanding. Cross-reference: load clean-code skill for naming conventions.

Introduce Parameter Object When: Long Parameter List smell — multiple parameters that always appear together.

// Before
func Search(query string, page, pageSize int, sortBy string, ascending bool) Results { ... }

// After
type SearchOptions struct {
    Query     string
    Page      int
    PageSize  int
    SortBy    string
    Ascending bool
}

func Search(opts SearchOptions) Results { ... }

Separate Query from Modifier (Command-Query Separation) When: a function both changes state and returns data.

// Bad: modifies AND returns
func (s *Stack) PopAndReturn() (Item, bool) {
    // removes and returns
}

// Good: separate
func (s *Stack) Peek() (Item, bool) { ... } // query only
func (s *Stack) Pop() bool { ... }          // command only

Dealing with Generalization

Extract Interface When: you have multiple implementations or want to enable testing with a test double.

// Before: depends on concrete type
type Service struct {
    db *postgres.DB
}

// After: depends on interface
type Store interface {
    Save(ctx context.Context, u User) error
}

type Service struct {
    store Store
}

Replace Inheritance with Delegation (Go: Embedding → Explicit Delegation) When: embedding is used but only some methods of the embedded type are needed.

Refactoring Sequence Dependencies

Apply in this order to avoid breaking changes:

  1. Extract Variable → then Extract Function → then Move Function
  2. Encapsulate Field → before other data refactorings
  3. Extract Function → before Move Function
  4. Rename → before Extract Interface

Go-Specific Refactoring Notes

Extracting interfaces

In Go, define the interface at the point of use, not at the implementation:

// The service package defines what it needs
package service

type UserStore interface {
    GetByID(ctx context.Context, id UserID) (User, error)
}

Error handling during refactoring

When extracting functions, preserve the error wrapping chain:

// Extracted function must wrap errors with its context
func validateOrder(o *Order) error {
    if len(o.Items) == 0 {
        return errors.New("order has no items")
    }
    return nil
}

// Caller wraps the context
if err := validateOrder(o); err != nil {
    return fmt.Errorf("process order: %w", err)
}

Struct options pattern (for long parameter lists)

Go idiom for optional parameters:

type ServerOption func(*Server)

func WithTimeout(d time.Duration) ServerOption {
    return func(s *Server) { s.timeout = d }
}

func NewServer(addr string, opts ...ServerOption) *Server {
    s := &Server{addr: addr, timeout: 30 * time.Second}
    for _, opt := range opts {
        opt(s)
    }
    return s
}

Refactoring Commit Strategy

Refactoring commits should be separate from feature commits:

  • refactor: extract validateOrder from processOrder
  • refactor: replace string email with Email value type
  • refactor: rename UserData to UserProfile

Never combine: feat: add discount + refactor: extract discount calculator

Verification Checklist

After each refactoring session:

  • go test ./... passes
  • go test -race ./... passes
  • No behavior change (same inputs produce same outputs)
  • Committed with a refactor: prefix commit message
  • Smells addressed are documented (if doing a review)

Cross-References

  • When applying Extract Function: load clean-code skill for naming conventions
  • When introducing interfaces: load solid skill for DIP and ISP guidance
  • When detecting smells: load code-review skill
  • Source: Martin Fowler, Refactoring: Improving the Design of Existing Code (2nd ed.)
  • Full catalog: https://refactoring.guru/refactoring