-
Notifications
You must be signed in to change notification settings - Fork 3
Closed as not planned
Description
🔍 Duplicate Code Pattern: Logger Initialization
Part of duplicate code analysis: #249
Summary
Three logger types (JSONLLogger, MarkdownLogger, FileLogger) implement nearly identical initialization patterns with only minor variations in error handling and post-initialization setup. This creates maintenance burden and risk of inconsistent behavior.
Duplication Details
Pattern: Init Function Structure
- Severity: High
- Occurrences: 3 instances
- Locations:
internal/logger/jsonl_logger.go(lines 38-64)internal/logger/markdown_logger.go(lines 27-56)internal/logger/file_logger.go(lines 28-63)
Code Sample from JSONLLogger:
func InitJSONLLogger(logDir, fileName string) error {
globalJSONLMu.Lock()
defer globalJSONLMu.Unlock()
if globalJSONLLogger != nil {
// Close existing logger
globalJSONLLogger.Close()
}
jl := &JSONLLogger{
logDir: logDir,
fileName: fileName,
}
// Try to initialize the log file
file, err := initLogFile(logDir, fileName, os.O_APPEND)
if err != nil {
return err
}
jl.logFile = file
jl.encoder = json.NewEncoder(file)
globalJSONLLogger = jl
return nil
}Code Sample from MarkdownLogger:
func InitMarkdownLogger(logDir, fileName string) error {
globalMarkdownMu.Lock()
defer globalMarkdownMu.Unlock()
if globalMarkdownLogger != nil {
// Close existing logger
globalMarkdownLogger.Close()
}
ml := &MarkdownLogger{
logDir: logDir,
fileName: fileName,
}
// Try to initialize the log file
file, err := initLogFile(logDir, fileName, os.O_TRUNC)
if err != nil {
// File initialization failed - set fallback mode
ml.useFallback = true
globalMarkdownLogger = ml
return nil
}
ml.logFile = file
ml.initialized = false
globalMarkdownLogger = ml
return nil
}Code Sample from FileLogger:
func InitFileLogger(logDir, fileName string) error {
globalLoggerMu.Lock()
defer globalLoggerMu.Unlock()
if globalFileLogger != nil {
// Close existing logger
globalFileLogger.Close()
}
fl := &FileLogger{
logDir: logDir,
fileName: fileName,
}
// Try to initialize the log file
file, err := initLogFile(logDir, fileName, os.O_APPEND)
if err != nil {
// File initialization failed - fallback to stdout
log.Printf("WARNING: Failed to initialize log file: %v", err)
log.Printf("WARNING: Falling back to stdout for logging")
fl.useFallback = true
fl.logger = log.New(os.Stdout, "", 0)
globalFileLogger = fl
return nil
}
fl.logFile = file
fl.logger = log.New(file, "", 0)
globalFileLogger = fl
return nil
}Impact Analysis
- Maintainability: Changes to initialization logic (e.g., adding validation, changing locking strategy) must be replicated across 3 files
- Bug Risk: Inconsistent error handling across loggers - JSONLLogger returns errors, while MarkdownLogger and FileLogger use fallback mode
- Code Bloat: ~80 lines of duplicated initialization code across 3 files
Refactoring Recommendations
-
Extract Generic Initialization Function
- Create a generic
initLogger[T]function incommon.gothat handles:- Mutex locking/unlocking
- Closing existing logger
- Creating logger struct
- Calling
initLogFilewith appropriate flags - Error handling with configurable fallback behavior
- Estimated effort: 2-3 hours
- Benefits:
- Single source of truth for initialization logic
- Consistent error handling across all loggers
- Easier to add new logger types
- Create a generic
-
Unified Error Handling Strategy
- Decide on consistent error handling: either all return errors or all use fallback
- Currently: JSONLLogger returns errors, while MarkdownLogger and FileLogger use fallback mode
- Estimated effort: 1 hour
- Benefits: Predictable behavior across all logger types
-
Type-Specific Customization via Interfaces
- Define a
LoggerInitializerinterface with methods likeSetupFile(file *os.File)andHandleInitError(err error) error - Each logger type implements this interface for its specific behavior
- Estimated effort: 1-2 hours
- Benefits: Maintains type-specific behavior while sharing common initialization code
- Define a
Implementation Checklist
- Review duplication findings with team
- Decide on error handling strategy (return error vs. fallback)
- Design generic initialization function signature
- Implement generic initialization in
common.go - Refactor JSONLLogger to use generic initialization
- Refactor MarkdownLogger to use generic initialization
- Refactor FileLogger to use generic initialization
- Update tests to verify behavior unchanged
- Verify no functionality broken
Parent Issue
See parent analysis report: #249
Related to #249
AI generated by Duplicate Code Detector