-
Notifications
You must be signed in to change notification settings - Fork 3
Closed as not planned
Closed as not planned
Copy link
Description
🔍 Duplicate Code Pattern: Dual Logging Functions (Md suffix)
Part of duplicate code analysis: #264
Summary
Four logging functions (LogInfoMd, LogWarnMd, LogErrorMd, LogDebugMd) follow an identical pattern where they call both the regular logger and the markdown logger. This creates ~50 lines of duplicated code that could be consolidated into a single higher-order function.
Duplication Details
Pattern: LogLevelMd Functions
- Severity: Medium
- Occurrences: 4 instances
- Locations:
internal/logger/markdown_logger.go(lines 167-221)
Code Sample:
// LogInfoMd logs to both regular and markdown loggers
func LogInfoMd(category, format string, args ...interface{}) {
// Log to regular logger
LogInfo(category, format, args...)
// Log to markdown logger
globalMarkdownMu.RLock()
defer globalMarkdownMu.RUnlock()
if globalMarkdownLogger != nil {
globalMarkdownLogger.Log(LogLevelInfo, category, format, args...)
}
}
// LogWarnMd logs to both regular and markdown loggers
func LogWarnMd(category, format string, args ...interface{}) {
// Log to regular logger
LogWarn(category, format, args...)
// Log to markdown logger
globalMarkdownMu.RLock()
defer globalMarkdownMu.RUnlock()
if globalMarkdownLogger != nil {
globalMarkdownLogger.Log(LogLevelWarn, category, format, args...)
}
}
// LogErrorMd and LogDebugMd follow the exact same patternAll four functions are structurally identical:
- Call the corresponding regular log function
- Acquire read lock on markdown logger
- Check if markdown logger exists
- Call markdown logger's Log method with appropriate level
Impact Analysis
- Maintainability: Medium impact - Changes to dual-logging behavior require 4 identical updates
- Bug Risk: Medium - If one function is updated differently, logging behavior becomes inconsistent
- Code Bloat: ~50 lines that could be reduced to ~15 lines
- Testing Overhead: Requires 4 separate tests for identical behavior
Refactoring Recommendations
1. Create Generic Dual Logger Function
- Extract common dual-logging pattern to a single function
- Estimated effort: 1-2 hours
- Benefits:
- Single source of truth for dual-logging behavior
- Easier to modify logging strategy (e.g., add JSONL logging)
- Reduced code size and test overhead
Suggested implementation:
// logToBoth logs to both file logger and markdown logger
func logToBoth(level LogLevel, category, format string, args ...interface{}) {
// Log to regular file logger
globalLoggerMu.RLock()
if globalFileLogger != nil {
globalFileLogger.Log(level, category, format, args...)
}
globalLoggerMu.RUnlock()
// Log to markdown logger
globalMarkdownMu.RLock()
if globalMarkdownLogger != nil {
globalMarkdownLogger.Log(level, category, format, args...)
}
globalMarkdownMu.RUnlock()
}
// Now all Md functions become one-liners
func LogInfoMd(category, format string, args ...interface{}) {
logToBoth(LogLevelInfo, category, format, args...)
}
func LogWarnMd(category, format string, args ...interface{}) {
logToBoth(LogLevelWarn, category, format, args...)
}
func LogErrorMd(category, format string, args ...interface{}) {
logToBoth(LogLevelError, category, format, args...)
}
func LogDebugMd(category, format string, args ...interface{}) {
logToBoth(LogLevelDebug, category, format, args...)
}2. Alternative: Logger Chain Pattern
- Implement a chain-of-responsibility pattern for multiple loggers
- Estimated effort: 3-4 hours
- Benefits:
- More flexible - can add/remove loggers dynamically
- Supports arbitrary number of loggers
- Cleaner architecture
Example:
type LoggerChain struct {
loggers []Logger
mu sync.RWMutex
}
func (c *LoggerChain) Log(level LogLevel, category, format string, args ...interface{}) {
c.mu.RLock()
defer c.mu.RUnlock()
for _, logger := range c.loggers {
logger.Log(level, category, format, args...)
}
}
// Usage
var markdownChain = &LoggerChain{
loggers: []Logger{globalFileLogger, globalMarkdownLogger},
}
func LogInfoMd(category, format string, args ...interface{}) {
markdownChain.Log(LogLevelInfo, category, format, args...)
}3. Usage Analysis Consideration
- Used in 13 locations across the codebase (see grep results)
- Primary usage in
internal/cmd/root.gofor startup/shutdown logging - Consider if markdown logging should be opt-in rather than always paired
Usage locations:
./internal/cmd/root.go:117
./internal/cmd/root.go:124
./internal/cmd/root.go:140
./internal/cmd/root.go:143
./internal/cmd/root.go:161
./internal/cmd/root.go:174
./internal/cmd/root.go:176
./internal/cmd/root.go:211
./internal/cmd/root.go:225
./internal/cmd/root.go:226
./internal/cmd/root.go:238
./internal/cmd/root.go:239
./internal/server/auth.go:24
./internal/server/auth.go:32
./internal/server/transport.go:113
./internal/mcp/connection.go:116
./internal/mcp/connection.go:125
./internal/mcp/connection.go:132
./internal/mcp/connection.go:141
Implementation Checklist
- Review current dual-logging usage patterns
- Decide between simple consolidation vs chain pattern
- Implement
logToBoth()helper function - Refactor all 4
Log*Md()functions to use helper - Update tests to cover consolidated behavior
- Verify all 19 usage sites still work correctly
- Consider adding JSONL to the chain if appropriate
- Run full test suite
Parent Issue
See parent analysis report: #264
Related to #264
AI generated by Duplicate Code Detector
Reactions are currently unavailable