[duplicate-code] Duplicate Code Pattern: Environment Variable Default Functions #582
Closed as not planned
Description
🔍 Duplicate Code Pattern: Environment Variable Default Functions
Part of duplicate code analysis: #581
Summary
Three nearly identical getDefault*() functions implement the same environment variable fallback pattern with minor variations. These functions check for an environment variable and return either the env value or a hardcoded default.
Duplication Details
Pattern: Environment Variable with Fallback
- Severity: Medium
- Occurrences: 3 instances
- 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 Samples
Pattern 1: String environment variable (used 2x)
// getDefaultLogDir - flags_logging.go:32-37
func getDefaultLogDir() string {
if envLogDir := os.Getenv("MCP_GATEWAY_LOG_DIR"); envLogDir != "" {
return envLogDir
}
return defaultLogDir
}
// getDefaultPayloadDir - flags_logging.go:41-46
func getDefaultPayloadDir() string {
if envPayloadDir := os.Getenv("MCP_GATEWAY_PAYLOAD_DIR"); envPayloadDir != "" {
return envPayloadDir
}
return defaultPayloadDir
}Pattern 2: Boolean environment variable with case-insensitive parsing
// getDefaultEnableDIFC - flags_difc.go:30-38
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: Each new environment-backed flag requires copying this pattern, increasing code volume
- Bug Risk: Inconsistent parsing logic (bool vs string) scattered across files
- Code Bloat: ~15-20 lines of similar code across 2 files
- Testing: Each function needs separate tests despite identical logic
Refactoring Recommendations
Option 1: Generic Environment Variable Helper (Recommended)
Extract common logic to a utility package with type-safe helpers:
// internal/envutil/envutil.go
package envutil
import (
"os"
"strings"
)
// GetStringOrDefault returns environment variable value or default
func GetStringOrDefault(envKey, defaultValue string) string {
if val := os.Getenv(envKey); val != "" {
return val
}
return defaultValue
}
// GetBoolOrDefault returns environment variable as bool or default
func GetBoolOrDefault(envKey string, defaultValue bool) bool {
val := os.Getenv(envKey)
if val == "" {
return defaultValue
}
switch strings.ToLower(val) {
case "1", "true", "yes", "on":
return true
case "0", "false", "no", "off":
return false
default:
return defaultValue
}
}Updated flag files:
// flags_logging.go
func getDefaultLogDir() string {
return envutil.GetStringOrDefault("MCP_GATEWAY_LOG_DIR", defaultLogDir)
}
func getDefaultPayloadDir() string {
return envutil.GetStringOrDefault("MCP_GATEWAY_PAYLOAD_DIR", defaultPayloadDir)
}
// flags_difc.go
func getDefaultEnableDIFC() bool {
return envutil.GetBoolOrDefault("MCP_GATEWAY_ENABLE_DIFC", defaultEnableDIFC)
}Benefits:
- Reduces ~40 lines to ~10 lines
- Centralizes environment variable parsing logic
- Makes boolean parsing consistent and testable
- Easy to extend with GetIntOrDefault, GetDurationOrDefault, etc.
Option 2: Inline with Comments
If keeping code local is preferred, at least add a comment referencing the pattern:
// getDefaultLogDir follows the standard env-or-default pattern.
// See also: getDefaultPayloadDir, getDefaultEnableDIFC
func getDefaultLogDir() string {
if envLogDir := os.Getenv("MCP_GATEWAY_LOG_DIR"); envLogDir != "" {
return envLogDir
}
return defaultLogDir
}Implementation Checklist
- Review duplication findings with team
- Decide on refactoring approach (Option 1 vs Option 2)
- If Option 1: Create
internal/envutilpackage with helpers - If Option 1: Add unit tests for
GetStringOrDefaultandGetBoolOrDefault - Update all 3
getDefault*()functions to use helpers - Update existing tests to verify behavior unchanged
- Document pattern in AGENTS.md or CONTRIBUTING.md
Estimated effort: 1-2 hours (create utility + update 3 call sites + tests)
Parent Issue
See parent analysis report: #581
Related to #581
AI generated by Duplicate Code Detector