tdd-refactor-step

TDD Refactor Step

Concrete guidance for the REFACTOR step of the RED-GREEN-REFACTOR cycle. Covers code smells to look for, safe refactoring techniques, test code cleanup, rubocop integration, and verification.

Contents

• How to use this skill
• Related skills
• Decision: refactor or skip
• Code smells checklist
• Refactoring techniques
• Test code refactoring
• Rubocop integration
• Verification
• Project-specific patterns
• Boundaries

How to use this skill

Invoke this skill during the REFACTOR step of the TDD cycle, after GREEN (all tests pass). This skill expands the guidance in the Development Workflow REFACTOR step.

Typical invocation:

I just got to green. Walk me through the REFACTOR step using
the TDD Refactor Step skill.

Related skills

• Development Workflow — parent TDD workflow; this skill supplements its REFACTOR step
• RSpec Unit Testing Standards — test conventions that refactored tests must follow
• PR Readiness Review — final quality gate that catches remaining quality issues

Decision: refactor or skip

Not every GREEN step needs refactoring. Skip when all of the following are true:

• No hardcoded values from the GREEN step remain
• No duplication was introduced between new and existing code
• No method exceeds ~10 lines
• No parameter list exceeds 3 positional parameters
• Rubocop reports no new offenses on changed files
• Test setup is not duplicated across examples

If any condition is false, proceed with the relevant technique below.

Code smells checklist

Check the code written or modified in this task for these smells, in priority order:

Limit yourself to smells in files you touched this task. Broader cleanup belongs in a separate task (add it during REPLAN).

Refactoring techniques

Apply the simplest technique that resolves the smell:

Extract method

Split a long method into a public method and one or more private helpers. Name the helper after what it does, not how:

# Before
def call(*, **)
  bound = args_definition.bind(*, **)
  objects = Array(bound.objects).map { |o| "#{o}\n" }.join
  with_stdin(objects) { |r| run_batch(bound, r) }
end

# After
def call(*, **)
  bound = args_definition.bind(*, **)
  with_stdin(stdin_content(bound)) { |r| run_batch(bound, r) }
end

private

def stdin_content(bound)
  Array(bound.objects).map { |o| "#{o}\n" }.join
end

Convert positional to keyword arguments

When a method accumulates optional trailing positional parameters:

# Before
def initialize(args, options, positionals, exec_names = [], flags = [])

# After
def initialize(args, options, positionals, exec_names: [], flags: [])

Update the single call site at the same time.

Replace conditional with guard clause

# Before
def validate(value)
  if value
    if value.is_a?(String)
      process(value)
    end
  end
end

# After
def validate(value)
  return unless value
  return unless value.is_a?(String)

  process(value)
end

Introduce constant

When a magic value appears in logic:

# Before
raise error if version < Git::Version.parse('2.28.0')

# After
MINIMUM_GIT_VERSION = Git::Version.parse('2.28.0')
raise error if version < MINIMUM_GIT_VERSION

Eliminate duplication with shared setup

When two methods share identical preamble or teardown, extract the shared part. If the duplication is in tests, see Test Code Refactoring below.

Test code refactoring

Test code deserves the same refactoring attention as production code:

Follow the RSpec Unit Testing Standards for the resulting test structure.

Rubocop integration

Run rubocop on changed files after refactoring:

bundle exec rubocop $(git diff --name-only HEAD)

Focus on:

• Metrics/MethodLength — methods over the configured limit
• Metrics/ParameterLists — too many parameters
• Metrics/AbcSize — complexity threshold
• Style cops — naming, formatting consistency

Auto-correct safe offenses when appropriate:

bundle exec rubocop -a $(git diff --name-only HEAD)

Do not auto-correct Metrics cops — those require structural changes (extract method, split class), not formatting fixes.

Verification

After refactoring, confirm:

1. Tests still pass: bundle exec rspec <spec_file> for the current task's test file(s)
2. No new rubocop offenses: bundle exec rubocop $(git diff --name-only HEAD)
3. Behavior unchanged: No new test was added during REFACTOR — if you need a new test, you skipped a RED step

If any test fails after refactoring, the refactoring changed behavior. Revert and try a smaller change.

Project-specific patterns

Patterns specific to this codebase that the REFACTOR step should enforce:

• Command classes should not contain parsing logic — if refactoring reveals parsing in a command class, flag it for extraction (separate task)
• Arguments DSL Bound metadata uses keyword arguments — if adding new metadata fields, follow the keyword argument pattern
• Error classes inherit from Git::Error — ensure new errors follow the hierarchy in lib/git/errors.rb
• freeze constants — all new constants should be frozen (CONSTANT = value.freeze)
• Private methods go below a single private keyword, not inline private def

Boundaries

Things the REFACTOR step must not do:

• Add new behavior — no new features, no new test cases
• Change public API — method signatures visible to users stay the same
• Touch unrelated files — scope to files modified in this task; add broader refactoring to the task list during REPLAN
• Optimize prematurely — clarity over performance unless profiling data exists
• Over-abstract — do not create a helper for something used exactly once