-
Notifications
You must be signed in to change notification settings - Fork 3
Open
Description
🔍 Duplicate Code Pattern: Environment Variable Default Getters
Part of duplicate code analysis: #602
Summary
Three nearly identical functions in the internal/cmd package follow the same pattern to retrieve default values from environment variables. This duplication creates maintenance burden and increases the risk of inconsistent behavior when environment variable handling needs to change.
Duplication Details
Pattern: Environment Variable Getter with Fallback
- Severity: High
- Occurrences: 3 instances
- Total Duplicated Lines: ~45 lines (15 lines × 3 functions)
- Locations:
internal/cmd/flags_difc.go(lines 30-38) -getDefaultEnableDIFC()internal/cmd/flags_logging.go(lines 32-37) -getDefaultLogDir()internal/cmd/flags_logging.go(lines 41-46) -getDefaultPayloadDir()
Code Sample from flags_logging.go:
// getDefaultLogDir returns the default log directory, checking MCP_GATEWAY_LOG_DIR
// environment variable first, then falling back to the hardcoded default
func getDefaultLogDir() string {
if envLogDir := os.Getenv("MCP_GATEWAY_LOG_DIR"); envLogDir != "" {
return envLogDir
}
return defaultLogDir
}
// getDefaultPayloadDir returns the default payload directory, checking MCP_GATEWAY_PAYLOAD_DIR
// environment variable first, then falling back to the hardcoded default
func getDefaultPayloadDir() string {
if envPayloadDir := os.Getenv("MCP_GATEWAY_PAYLOAD_DIR"); envPayloadDir != "" {
return envPayloadDir
}
return defaultPayloadDir
}Code Sample from flags_difc.go:
// getDefaultEnableDIFC returns the default DIFC setting, checking MCP_GATEWAY_ENABLE_DIFC
// environment variable first, then falling back to the hardcoded default (false)
func getDefaultEnableDIFC() bool {
if envDIFC := os.Getenv("MCP_GATEWAY_ENABLE_DIFC"); envDIFC != "" {
switch strings.ToLower(envDIFC) {
case "1", "true", "yes", "on":
return true
}
}
return defaultEnableDIFC
}Impact Analysis
- Maintainability: Any change to environment variable handling (e.g., adding validation, logging, or error handling) must be replicated across all 3 functions
- Bug Risk: High - Changes to one function might not be consistently applied to others, leading to divergent behavior
- Code Bloat: Medium - ~45 lines of duplicated logic that could be reduced to ~20 lines with a shared utility
- Testing: Each function requires separate test cases, increasing test maintenance burden
Refactoring Recommendations
Option 1: Generic Environment Variable Getter (Recommended)
Extract common logic into a generic utility function in internal/cmd/flags.go:
// getEnvWithDefault returns the value of an environment variable or a default value
func getEnvWithDefault(envKey string, defaultValue string) string {
if envVal := os.Getenv(envKey); envVal != "" {
return envVal
}
return defaultValue
}
// getEnvBoolWithDefault returns a boolean from an environment variable or a default value
func getEnvBoolWithDefault(envKey string, defaultValue bool) bool {
if envVal := os.Getenv(envKey); envVal != "" {
switch strings.ToLower(envVal) {
case "1", "true", "yes", "on":
return true
case "0", "false", "no", "off":
return false
}
}
return defaultValue
}Then refactor existing functions:
// In flags_logging.go
func getDefaultLogDir() string {
return getEnvWithDefault("MCP_GATEWAY_LOG_DIR", defaultLogDir)
}
func getDefaultPayloadDir() string {
return getEnvWithDefault("MCP_GATEWAY_PAYLOAD_DIR", defaultPayloadDir)
}
// In flags_difc.go
func getDefaultEnableDIFC() bool {
return getEnvBoolWithDefault("MCP_GATEWAY_ENABLE_DIFC", defaultEnableDIFC)
}Benefits:
- Reduces ~45 lines to ~20 lines total
- Centralizes environment variable handling logic
- Makes adding logging/validation easier (single point of change)
- Improves testability (test utility functions once)
Estimated Effort: 1-2 hours
- Create utility functions
- Refactor existing code
- Update tests
Option 2: Inline Environment Variable Access
If the getters are only called once, consider inlining them at the call site to reduce function call overhead.
Benefits:
- Eliminates getter functions entirely
- More direct code
Drawbacks:
- Makes future changes harder
- Less testable
Implementation Checklist
- Review duplication findings with team
- Decide on refactoring approach (Option 1 recommended)
- Create
getEnvWithDefaultandgetEnvBoolWithDefaultutility functions ininternal/cmd/flags.go - Add unit tests for new utility functions
- Refactor
getDefaultLogDir,getDefaultPayloadDir, andgetDefaultEnableDIFCto use utilities - Update existing tests (should pass without changes if behavior is preserved)
- Run
make agent-finishedto verify all checks pass - Document the pattern for future flag additions
Parent Issue
See parent analysis report: #602
Related to #602
AI generated by Duplicate Code Detector
- expires on Feb 10, 2026, 10:24 AM UTC