Skip to content

chore(writer) Refactor writer package with strategy pattern architecture#395

Open
cduggn wants to merge 2 commits into
mainfrom
claude/refactor-writer-package-SxOxN
Open

chore(writer) Refactor writer package with strategy pattern architecture#395
cduggn wants to merge 2 commits into
mainfrom
claude/refactor-writer-package-SxOxN

Conversation

@cduggn

@cduggn cduggn commented Jan 7, 2026

Copy link
Copy Markdown
Owner

This is Phase 1 of the writer package refactoring, focused on removing legacy code and consolidating the modern strategy pattern architecture.

Changes:

  • Deleted legacy mapper functions from writer.go (CostAndUsageToVectorMapper, CostAndUsageToStdoutMapper, CostAndUsageToCSVMapper, etc.)
  • Removed old table implementations by deleting stdout.go entirely
  • Deleted csv.go with legacy CSV helper functions
  • Removed legacy wrapper types from writers.go (GenericStdoutPrinter, GenericCsvPrinter, GenericChartPrinter, GenericPineconePrinter)
  • Replaced with a cleaner PrinterAdapter that consolidates all legacy interface compatibility in one place
  • Cleaned up chart.go by removing WriteToChart function and unused imports
  • Removed unused global variables (csvFileName, csvHeader)
  • Added comprehensive package documentation explaining the strategy pattern architecture

Impact:

  • Reduced code by ~200 lines
  • Eliminated code duplication between old and new implementations
  • Clearer separation of concerns with dedicated transformers and renderers
  • All existing functionality preserved through PrinterAdapter
  • All tests passing, build successful

The package now has a clean foundation built on:

  • Transformer interface: converts domain data to output formats
  • Renderer interface: writes formatted data to destinations
  • CompositeWriter: combines transformer + renderer
  • Factory functions in writers.go for creating specific writer types

Next phases will focus on improving the factory pattern and adding dependency injection for better testability.

PR Checklist

  • Read the Contributing documentation.
  • Name your Pull Request title clearly, concisely, and prefixed with the name of the primarily affected package you changed according to Good commit messages (such as memory: add interfaces for X, Y or util: add whizzbang helpers).
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. Fixes #123).
  • Describes the source of new concepts.
  • References existing implementations as appropriate.
  • Contains test coverage for new functions.
  • Passes all golangci-lint checks.

claude added 2 commits January 7, 2026 23:06
…e architecture

This is Phase 1 of the writer package refactoring, focused on removing legacy
code and consolidating the modern strategy pattern architecture.

Changes:
- Deleted legacy mapper functions from writer.go (CostAndUsageToVectorMapper,
  CostAndUsageToStdoutMapper, CostAndUsageToCSVMapper, etc.)
- Removed old table implementations by deleting stdout.go entirely
- Deleted csv.go with legacy CSV helper functions
- Removed legacy wrapper types from writers.go (GenericStdoutPrinter,
  GenericCsvPrinter, GenericChartPrinter, GenericPineconePrinter)
- Replaced with a cleaner PrinterAdapter that consolidates all legacy
  interface compatibility in one place
- Cleaned up chart.go by removing WriteToChart function and unused imports
- Removed unused global variables (csvFileName, csvHeader)
- Added comprehensive package documentation explaining the strategy pattern
  architecture

Impact:
- Reduced code by ~200 lines
- Eliminated code duplication between old and new implementations
- Clearer separation of concerns with dedicated transformers and renderers
- All existing functionality preserved through PrinterAdapter
- All tests passing, build successful

The package now has a clean foundation built on:
- Transformer interface: converts domain data to output formats
- Renderer interface: writes formatted data to destinations
- CompositeWriter: combines transformer + renderer
- Factory functions in writers.go for creating specific writer types

Next phases will focus on improving the factory pattern and adding dependency
injection for better testability.
…ons, and type-safe factory

This commit implements Phase 2 and Phase 3 of the writer refactoring:
- Dependency injection for configuration
- Functional options pattern
- Type-safe factory with improved API

Changes:
- Created Config struct for centralizing writer configuration (config.go)
- Implemented functional options pattern (WithOutputDir, WithSortBy, WithVectorDBConfig, etc.)
- Added WriterFactory for type-safe writer creation (factory.go)
- Updated all renderers to accept Config via dependency injection
- Modified factory functions to use variadic options instead of individual parameters
- Removed hardcoded values (OutputDir, API keys) from renderers
- Updated PrinterAdapter to use functional options for backward compatibility

Architecture improvements:
- Renderers no longer depend on global state
- Configuration is explicitly passed through the call chain
- Better testability via injected dependencies
- Type-safe factory methods eliminate runtime type assertions for new code
- Flexible configuration through functional options

Impact:
- All tests passing
- Build successful
- Backward compatible through PrinterAdapter
- New code can use type-safe factory functions
- Better separation of concerns and testability

Example usage of new API:
```go
// Using functional options
writer := writer.NewCostUsageCSVWriter(
    writer.WithSortBy("date"),
    writer.WithOutputDir("/tmp"),
)
writer.Write(data)

// Using factory
factory := writer.NewWriterFactory(
    writer.WithSortBy("amount"),
    writer.WithVectorDBConfig(openAIKey, pineconeKey, index),
)
factory.WriteCostUsage(writer.FormatCSV, data)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants