🔍 Duplicate Code Pattern: Logger Initialization Functions

Part of duplicate code analysis: #581

Summary

Three logger types (FileLogger, JSONLLogger, MarkdownLogger) have similar initialization functions that call a shared generic initLogger() helper. The duplication is intentional according to code comments in common.go, which explicitly documents this pattern as a design decision.

Duplication Details

Pattern: Logger Initialization with Generic Helper

• Severity: Low (intentional design)
• Occurrences: 3 instances
• Locations:
  • internal/logger/file_logger.go (lines 31-46) - InitFileLogger()
  • internal/logger/jsonl_logger.go (lines 40-55) - InitJSONLLogger()
  • internal/logger/markdown_logger.go (lines 29-44) - InitMarkdownLogger()

Code Sample

All three follow the same pattern:

// InitFileLogger - file_logger.go:31-46
func InitFileLogger(logDir, fileName string) error {
	logger, err := initLogger(
		logDir, fileName, os.O_APPEND,
		// Setup function: configure the logger after file is opened
		func(file *os.File, logDir, fileName string) (*FileLogger, error) {
			fl := &FileLogger{
				logDir:   logDir,
				fileName: fileName,
				logFile:  file,
				logger:   log.New(file, "", 0),
			}
			return fl, nil
		},
		// Error handler: fallback to stdout
		func(err error, logDir, fileName string) (*FileLogger, error) {
			// ... fallback logic ...
		},
	)
	// ... store in global ...
}

Impact Analysis

Why This Duplication is Acceptable:

From internal/logger/common.go:67-72:

// This pattern is intentionally duplicated across logger types rather than abstracted:
//   - It's a standard Go idiom for wrapper methods
//   - The duplication is minimal (5-14 lines per type)
//   - Each logger can customize cleanup as needed
//   - The shared closeLogFile() helper eliminates complex logic duplication

Key Points:

• The complex logic IS shared via the generic initLogger() function (line 161)
• Each logger type has type-specific setup and error handling
• Duplication is ~15-20 lines per logger type
• The pattern is well-documented and intentional
• Alternative abstraction would add complexity without clear benefit

Refactoring Recommendations

Recommendation: No Action Required

This duplication pattern should NOT be refactored because:

1. Already Optimized: The complex initialization logic is abstracted into the generic initLogger[T closableLogger]() function
2. Type Safety: Each Init function returns a specific logger type, maintaining type safety
3. Customization: Each logger has type-specific setup (e.g., FileLogger needs log.New(), JSONLLogger needs json.NewEncoder())
4. Go Idioms: This follows standard Go patterns - wrapper functions with minimal duplication
5. Clear Documentation: The design decision is explicitly documented in code comments

If Changes Are Made in Future

If someone insists on further abstraction, consider:

Anti-Pattern Warning:

// DON'T DO THIS - loses type safety and readability
func InitAnyLogger(loggerType string, logDir, fileName string) (interface{}, error) {
	switch loggerType {
	case "file":
		return InitFileLogger(logDir, fileName)
	// ...
	}
}

Better: Keep current pattern but ensure consistency:

• All Init functions follow the same structure
• All use initLogger() helper
• All document their specific setup/fallback behavior

Implementation Checklist

• Review duplication findings - Analysis complete
• Prioritize refactoring - No refactoring needed (intentional design)
• Close this sub-issue as "Won't Fix" with reference to design documentation
• Optionally: Add a note to AGENTS.md about this intentional pattern

Estimated effort: 0 hours (no changes needed)

Design Documentation Reference

See internal/logger/common.go:67-72 for the explicit design decision documentation:

This pattern is intentionally duplicated across logger types rather than abstracted

Parent Issue

See parent analysis report: #581

Note to Reviewers: This issue is included for completeness in the duplication analysis. The pattern was detected by automated analysis but is actually a well-documented design decision. Consider closing as "Won't Fix" or "Working as Intended".
Related to #581

AI generated by Duplicate Code Detector