Skip to content

Deduplicate column argument parsing between extractors #162

Description

@eitoball

Summary

Extract shared column-argument parsing logic from DataExtractor and HeaderExtractor into a single code path.

Problem

Comma::DataExtractor#method_missing and Comma::HeaderExtractor#method_missing implement nearly identical branching over argument shapes:

Argument shape Meaning
(none) Bare method call on the instance
String Custom header label; data still from the method
Symbol Traverse association, call method on associated object
Hash Association traversal with explicit header keys

Only the per-column handler differs (extract value vs. humanize header). Duplication means:

  • Bug fixes must be applied twice
  • New column types require two parallel changes
  • Higher cognitive load for contributors

Relevant files:

  • lib/comma/data_extractor.rb
  • lib/comma/header_extractor.rb
  • lib/comma/extractor.rb (base class)

Proposed approach

Introduce shared parsing on Comma::Extractor, e.g.:

def each_column_definition(sym, args)
  if args.blank?
    yield ColumnDefinition.new(source: :instance, method: sym)
    return
  end

  args.each do |arg|
    yield parse_column_arg(sym, arg)
  end
end

Each subclass supplies callbacks:

  • DataExtractor — resolve value (instance, association, block transform)
  • HeaderExtractor — resolve label (humanizer, custom string, association class)

Keep __static_column__ as separate entry points; data and header semantics differ intentionally.

Acceptance criteria

  • All specs in spec/comma/data_extractor_spec.rb pass unchanged (behavior preserved)
  • All specs in spec/comma/header_extractor_spec.rb pass unchanged
  • All specs in spec/comma/comma_spec.rb pass unchanged
  • Argument parsing logic exists in one place (DRY)
  • No change to the public DSL

Out of scope

  • Replacing method_missing with an explicit builder API (would be a major version change)

Labels (suggested)

refactor, good first issue (after #1 or in parallel if comfortable with extractors)

Depends on

Optional: #1 (STI fix) — unrelated but reduces noise when running AR specs during this work.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions