Craftsman Refactor

---
name: craftsman-refactor
description: Systematic refactoring with behavior preservation. Use when improving existing code structure, reducing technical debt, or when code smells are detected.
version: 1.0.0
platforms: [macos, linux]
metadata:
  hermes:
    tags: [craftsmanship, refactor]
    category: devops
    requires_toolsets: [terminal]
---

<!-- Generated by scripts/export-hermes-skills.sh from skills/refactor/SKILL.md. Edit the source, then re-run the export. -->

## When to Use

Systematic refactoring with behavior preservation. Use when improving existing code structure, reducing technical debt, or when code smells are detected.


# the craftsman-refactor skill - Systematic Refactoring

## Outcome Contract

- **Outcome**: improved structure with behaviour provably unchanged.
- **Done when**: the test suite passed before the refactor, passes after, and no test was modified to make it pass.
- **Evidence**: the test run before, the test run after, and the smell that motivated the change.

You are a **Senior Engineer** obsessed with clean code. You refactor methodically, not randomly.

## Principles

| Principle | Meaning |
|-----------|---------|
| Behavior UNCHANGED | Tests must pass before AND after |
| INCREMENTAL changes | One refactoring at a time |
| YAGNI | No over-engineering |
| Measure first | Don't optimize without evidence |

## Code Smells Catalog

### Structural Smells

| Smell | Detection | Refactoring |
|-------|-----------|-------------|
| **Long Method** | >30 lines | Extract Method |
| **Large Class** | >200 lines | Extract Class |
| **Long Parameter List** | >3 params | Introduce Parameter Object |
| **Primitive Obsession** | `string $email` | Extract Value Object |
| **Data Clumps** | Same params together | Extract Class |

### Coupling Smells

| Smell | Detection | Refactoring |
|-------|-----------|-------------|
| **Feature Envy** | Uses other object's data | Move Method |
| **Inappropriate Intimacy** | Classes know too much | Extract Interface |
| **Message Chains** | `a.b().c().d()` | Hide Delegate |
| **Middle Man** | Class only delegates | Remove Middle Man |

### Change Smells

| Smell | Detection | Refactoring |
|-------|-----------|-------------|
| **Divergent Change** | One class, many reasons to change | Extract Class |
| **Shotgun Surgery** | One change, many classes | Move Method/Field |

### Dispensables

| Smell | Detection | Refactoring |
|-------|-----------|-------------|
| **Dead Code** | Unused code | Delete |
| **Speculative Generality** | Unused abstractions | Inline/Delete |
| **Comments** | Explaining what, not why | Rename/Extract |

## Process

### Step 0: Safety Net First (GATE)

**No net, no refactor.** Before touching anything, establish the safety net:

1. Run the test suite for the target code.
2. **Tests green** -> proceed to Step 1.
3. **Tests red, absent, or too slow to trust** -> do NOT refactor blind. Route to `the craftsman-legacy skill cover` to build a characterization / golden-master net around the current behavior, then resume here. See `references/legacy-characterization-testing.md`.

Refactoring untested code without a net is not refactoring; it is gambling. This gate is non-negotiable and is the behavioral link between `/refactor` and the `/legacy` command.

### Step 1: Identify Smells

Analyze code and list smells with severity:

```markdown
## Smell Analysis: [File/Component]

| # | Smell | Location | Severity | Refactoring |
|---|-------|----------|----------|-------------|
| 1 | Primitive Obsession | User.php:45 | High | Extract Email VO |
| 2 | Long Method | OrderService.php:120 | Medium | Extract Method |
| 3 | Dead Code | Utils.php:200-250 | Low | Delete |
```

### Step 2: Prioritize

Order by: `(Technical Debt) × (Change Frequency)`

```markdown
## Priority Order

1. **Email Value Object** - High debt, frequently modified
2. **Extract calculateTotal** - Medium debt, core logic
3. **Remove dead code** - Low debt, never touched
```

### Step 3: Plan

For each smell, propose the refactoring:

```markdown
## Refactoring Plan

### 1. Extract Email Value Object

**From:**
```php
final class User
{
    private string $email;

    public function setEmail(string $email): void
    {
        if (!filter_var($email, FILTER_VALIDATE_EMAIL)) {
            throw new \InvalidArgumentException('Invalid email');
        }
        $this->email = $email;
    }
}
```

**To:**
```php
final class Email
{
    private function __construct(private readonly string $value) {}

    public static function fromString(string $value): self
    {
        if (!filter_var($value, FILTER_VALIDATE_EMAIL)) {
            throw InvalidEmailException::invalid($value);
        }
        return new self($value);
    }
}

final class User
{
    private Email $email;

    public function changeEmail(Email $email): void
    {
        $this->email = $email;
        $this->record(new UserEmailChanged($this->id, $email));
    }
}
```

**Impact:**
- Type safety improved
- Validation centralized
- Behavior method instead of setter

---

**Proceed with this refactoring?** [Wait for confirmation]
```

### Step 4: Execute

One refactoring at a time. After EACH change:

```bash
# Run tests
vendor/bin/phpunit

# Run static analysis
vendor/bin/phpstan analyse

# Verify behavior unchanged
```

#### Mikado Mode (multi-file campaigns)

When a refactoring ripples across many files and each attempt uncovers new prerequisites, switch to the **Mikado Method** (see `references/refactoring-mikado-method.md`):

1. Write the goal; mark the starting point (below); start a ~10 minute timer; attempt it directly.
2. Not done when the timer rings? Write the blocking prerequisites as subgoals, throw the attempt away (below: keep the graph, drop the code), tackle a leaf first.
3. Deliver from the leaves inward, committing and shipping each, and mark the new starting point after each commit; the goal falls out for free at the end.

You run in the user's checkout, where uncommitted work may predate the attempt. Revert only what the attempt changed, never `git reset --hard`: it would destroy that work too. An attempt stages nothing and commits nothing.

**Mark the starting point** (before the first attempt, and after each delivered commit):

```bash
cd "$(git rev-parse --show-toplevel)"
MIKADO_INDEX="$(git rev-parse --git-path mikado.index)"
cp "$(git rev-parse --git-path index)" "$MIKADO_INDEX" 2>/dev/null || rm -f "$MIKADO_INDEX"
GIT_INDEX_FILE="$MIKADO_INDEX" git add -A
git update-ref refs/mikado/start "$(GIT_INDEX_FILE="$MIKADO_INDEX" git write-tree)"
```

**Throw the attempt away** (the tree returns to the mark exactly: edits reversed, created files removed, deleted files restored, earlier uncommitted work kept):

```bash
cd "$(git rev-parse --show-toplevel)"
MIKADO_INDEX="$(git rev-parse --git-path mikado.index)"
GIT_INDEX_FILE="$MIKADO_INDEX" git add -A
MIKADO_NOW="$(GIT_INDEX_FILE="$MIKADO_INDEX" git write-tree)"
git diff --quiet refs/mikado/start "$MIKADO_NOW" || git diff --binary refs/mikado/start "$MIKADO_NOW" | git apply -R
```

When the goal is reached, drop the mark (`git update-ref -d refs/mikado/start`). The why and the edge cases are in `references/refactoring-mikado-method.md` (Reverting Only the Attempt).

Persist the graph across sessions in `.craftsman/mikado.json` (atomic write: `tempfile.mkstemp()` + `os.rename()`), so an interruption never loses the map. Discipline: **revert, do not fix** a failing attempt. Render the final graph as Mermaid in the session summary.

### Step 5: Validate

```markdown
## Refactoring Complete

### Before
- File: User.php (245 lines)
- Smells: Primitive obsession, setter abuse
- Coupling: High (validation spread)

### After
- Files: User.php (180 lines), Email.php (35 lines)
- Smells: None detected
- Coupling: Low (validation encapsulated)

### Verification
```
vendor/bin/phpunit
OK (42 tests, 98 assertions)

vendor/bin/phpstan analyse
[OK] No errors
```

### Behavior Change
None. All tests pass without modification.
```

## Knowledge References

For detailed techniques and legacy strategy, read these files:
- `references/refactoring-techniques.md` - the full Fowler catalog (Extract, Move, Split Phase, Pipeline, CQS)
- `references/refactoring-refactoring-campaigns.md` - hotspots (churn x complexity), prioritization
- `references/refactoring-mikado-method.md` - safe multi-file change via discovery/delivery
- `references/legacy-legacy-techniques.md` - seams, subclass & override, wrap & sprout
- `references/legacy-characterization-testing.md` - golden master net before refactoring untested code
- `references/legacy-strangler-fig.md` - branch-by-abstraction, incremental cutover

## Refactoring Catalog

### Extract Method
```php
// Before
public function processOrder(): void
{
    // validate
    if ($this->items->isEmpty()) { throw ... }
    if ($this->total() < 0) { throw ... }

    // calculate
    $subtotal = 0;
    foreach ($this->items as $item) {
        $subtotal += $item->price() * $item->quantity();
    }
    // ... 50 more lines
}

// After
public function processOrder(): void
{
    $this->validate();
    $subtotal = $this->calculateSubtotal();
    // ...
}

private function validate(): void { ... }
private function calculateSubtotal(): Money { ... }
```

### Extract Value Object
```php
// Before
private string $email;
private string $phone;
private int $amountCents;

// After
private Email $email;
private Phone $phone;
private Money $amount;
```

### Replace Conditional with Polymorphism
```php
// Before
public function calculateFee(): Money
{
    return match ($this->type) {
        'premium' => $this->amount->multiply(0.01),
        'standard' => $this->amount->multiply(0.02),
        'basic' => $this->amount->multiply(0.05),
    };
}

// After
interface FeeCalculator
{
    public function calculate(Money $amount): Money;
}

final class PremiumFeeCalculator implements FeeCalculator { ... }
final class StandardFeeCalculator implements FeeCalculator { ... }
```

## Output Format

```markdown
## Refactoring: [Target]

### Smells Detected
| Smell | Location | Severity | Action |
|-------|----------|----------|--------|
| ... | ... | ... | ... |

### Refactoring Plan
1. [First refactoring] - [Impact]
2. [Second refactoring] - [Impact]

### Execution
[Step-by-step changes with diffs]

### Validation
- Tests: ✅ All pass
- Static analysis: ✅ Clean
- Behavior: ✅ Unchanged
```

## Bias Protection

**Over-optimization:** "Let's also abstract this..."
→ YAGNI. Only fix identified smells. Don't add complexity.

**Scope creep:** "While refactoring, let's add this feature..."
→ Refactoring ≠ Feature work. Separate concerns.

## References

- [refactoring.guru](https://refactoring.guru/refactoring)
- [refactoring.com](https://refactoring.com/)

> "Any fool can write code that a computer can understand. Good programmers write code that humans can understand." - Martin Fowler