-
-
Notifications
You must be signed in to change notification settings - Fork 0
Open
Labels
Description
Problem
The Formatter interface is defined in two places, causing unnecessary duplication.
Current Duplication
Definition 1: pkg/formatter/formatter.go
type Formatter interface {
Format(line, stream string) string
}Definition 2: pkg/processor/processor.go:36-39
type Formatter interface {
Format(line, stream string) string
}Issue
Having the same interface in multiple packages:
- ❌ Violates DRY principle
- ❌ Requires updates in multiple places if signature changes
- ❌ Confusing for maintainers (which one is authoritative?)
- ❌ Unnecessary duplication
Proposed Solution
Keep interface in pkg/formatter (the implementation package) and import it in pkg/processor.
Step 1: Remove from processor
// pkg/processor/processor.go
// DELETE THIS:
// type Formatter interface {
// Format(line, stream string) string
// }
// Import formatter package instead
import (
"github.com/sgaunet/logwrap/pkg/formatter"
)
type Processor struct {
config *config.Config
formatter formatter.Formatter // Use formatter.Formatter type
// ...
}Step 2: Ensure formatter package exports interface
// pkg/formatter/formatter.go
// Formatter is the interface for formatting log lines.
//
// Implementations must be safe for concurrent use by multiple goroutines.
type Formatter interface {
// Format adds configured prefix to a log line.
//
// Parameters:
// - line: The raw log line to format
// - stream: The stream name ("stdout" or "stderr")
//
// Returns formatted line with prefix, timestamp, log level, etc.
Format(line, stream string) string
}Step 3: Update processor constructor
// pkg/processor/processor.go
// New creates a new Processor.
//
// The formatter parameter must implement the formatter.Formatter interface.
func New(cfg *config.Config, f formatter.Formatter) *Processor {
return &Processor{
config: cfg,
formatter: f,
// ...
}
}Benefits
Code Quality:
- Single source of truth for interface
- Easier to maintain and modify
- Clearer package responsibilities
Maintainability:
- Interface changes only need one update
- Less risk of inconsistency
- Clearer dependency direction (processor depends on formatter)
Implementation Checklist
- Remove
Formatterinterface frompkg/processor/processor.go - Import
pkg/formatterin processor package - Update
Processorstruct to useformatter.Formatter - Update
New()constructor signature - Verify all tests still pass
- Update package documentation if needed
Testing
Ensure existing tests still work:
# Run all tests
go test ./pkg/processor ./pkg/formatter
# Verify no compilation errors
go build ./cmd/logwrapNo test changes should be needed - this is just a refactor.
Alternative: Keep in Processor
If we wanted to keep interface in processor (not recommended):
// pkg/processor/processor.go
// Formatter is the interface for formatters used by the processor.
type Formatter interface {
Format(line, stream string) string
}
// pkg/formatter/formatter.go - remove interface, just implement it
type DefaultFormatter struct {
// ...
}
func (f *DefaultFormatter) Format(line, stream string) string {
// Implementation
}Why not recommended:
- Formatter package should own its interface (Go best practice)
- Makes formatter package harder to use independently
- Reversed dependency (formatter would depend on processor interface)
Design Pattern
This follows the dependency inversion principle:
- High-level module (processor) depends on abstraction (Formatter interface)
- Low-level module (formatter) implements abstraction
- Interface should be in the package that defines it (formatter)
Related Issues
- Refactor redundant validation logic in config package #11 - Refactor validation logic (similar DRY improvements)
- Add comprehensive package-level documentation #14 - Package documentation (document interface ownership)
References
- Go interfaces: https://go.dev/tour/methods/9
- Effective Go: Interfaces
- Go Proverbs: "Accept interfaces, return concrete types"
Reactions are currently unavailable