🔍 Duplicate Code Pattern: Flag Environment Variable Getters
Part of duplicate code analysis: #797
Summary
Multiple getDefault*() functions in the internal/cmd package follow an identical pattern for retrieving configuration values from environment variables with fallback to hardcoded defaults. This pattern is repeated 4 times across flag files with only minor variations.
Duplication Details
Pattern: Environment Variable Getter Functions
- Severity: Medium
- Occurrences: 4 instances (3 in flags_logging.go, 1 in flags_difc.go)
- Locations:
internal/cmd/flags_logging.go:
getDefaultLogDir() (lines 36-41)
getDefaultPayloadDir() (lines 45-50)
getDefaultPayloadSizeThreshold() (lines 54-64)
internal/cmd/flags_difc.go:
getDefaultEnableDIFC() (lines 30-38)
Duplicated Structure
All functions follow the same pattern:
// Pattern 1: String environment variable
func getDefault(Name)() string {
if env(Name) := os.Getenv("MCP_GATEWAY_(NAME)"); env(Name) != "" {
return env(Name)
}
return default(Name)
}
// Pattern 2: Boolean environment variable
func getDefault(Name)() bool {
if env(Name) := os.Getenv("MCP_GATEWAY_(NAME)"); env(Name) != "" {
switch strings.ToLower(env(Name)) {
case "1", "true", "yes", "on":
return true
}
}
return default(Name)
}
// Pattern 3: Integer environment variable with validation
func getDefault(Name)() int {
if env(Name) := os.Getenv("MCP_GATEWAY_(NAME)"); env(Name) != "" {
var value int
if _, err := fmt.Sscanf(env(Name), "%d", &value); err == nil && value > 0 {
return value
}
}
return default(Name)
}
Code Examples
String Getter (getDefaultLogDir):
func getDefaultLogDir() string {
if envLogDir := os.Getenv("MCP_GATEWAY_LOG_DIR"); envLogDir != "" {
return envLogDir
}
return defaultLogDir
}
String Getter (getDefaultPayloadDir) - Nearly identical:
func getDefaultPayloadDir() string {
if envPayloadDir := os.Getenv("MCP_GATEWAY_PAYLOAD_DIR"); envPayloadDir != "" {
return envPayloadDir
}
return defaultPayloadDir
}
Integer Getter (getDefaultPayloadSizeThreshold):
func getDefaultPayloadSizeThreshold() int {
if envThreshold := os.Getenv("MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD"); envThreshold != "" {
var threshold int
if _, err := fmt.Sscanf(envThreshold, "%d", &threshold); err == nil && threshold > 0 {
return threshold
}
}
return defaultPayloadSizeThreshold
}
Boolean Getter (getDefaultEnableDIFC):
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
- Medium Risk: New flags require copy-paste of getter pattern
- Boilerplate Heavy: Each new configuration value needs 6-11 lines of getter code
- Validation Inconsistency: Integer validation inline, boolean validation via switch, no validation for strings
Bug Risk
- Low-Medium Risk: Inconsistent validation approaches could lead to bugs
- Example: Integer validation checks
> 0, but what about negative values elsewhere?
- Missing Features: No support for default value overrides, validation error messages
Code Bloat
- Total Lines: ~40 lines of similar getter functions
- Future Growth: Every new flag adds 6-11 lines of duplicated code
Refactoring Recommendations
1. Create Generic Environment Getter Utility
Effort: Low-Medium (2-4 hours)
Extract common pattern into type-safe generic utility:
// Package envutil provides utilities for reading configuration from environment variables
package envutil
import (
"fmt"
"os"
"strconv"
"strings"
)
// GetEnvString returns string value from env var or default
func GetEnvString(envKey, defaultValue string) string {
if value := os.Getenv(envKey); value != "" {
return value
}
return defaultValue
}
// GetEnvInt returns integer value from env var or default
// Validates that value is positive (> 0)
func GetEnvInt(envKey string, defaultValue int) int {
if envValue := os.Getenv(envKey); envValue != "" {
if value, err := strconv.Atoi(envValue); err == nil && value > 0 {
return value
}
}
return defaultValue
}
// GetEnvBool returns boolean value from env var or default
// Accepts: "1", "true", "yes", "on" (case-insensitive)
func GetEnvBool(envKey string, defaultValue bool) bool {
if envValue := os.Getenv(envKey); envValue != "" {
switch strings.ToLower(envValue) {
case "1", "true", "yes", "on":
return true
case "0", "false", "no", "off":
return false
}
}
return defaultValue
}
Usage - Simplifies flag getters to 1 line:
// Before (6 lines)
func getDefaultLogDir() string {
if envLogDir := os.Getenv("MCP_GATEWAY_LOG_DIR"); envLogDir != "" {
return envLogDir
}
return defaultLogDir
}
// After (1 line)
func getDefaultLogDir() string {
return envutil.GetEnvString("MCP_GATEWAY_LOG_DIR", defaultLogDir)
}
Benefits:
- Reduces getter functions from 6-11 lines to 1 line each
- Consistent validation logic across all configuration
- Centralized error handling and type conversion
- Easy to add logging/debugging for env var reads
- Reduces total code by ~35 lines
2. Add Validation and Logging
Effort: Low (1-2 hours)
Enhance utility with validation and debug logging:
// GetEnvIntWithValidation returns integer with custom validation
func GetEnvIntWithValidation(envKey string, defaultValue int, validator func(int) bool) int {
if envValue := os.Getenv(envKey); envValue != "" {
if value, err := strconv.Atoi(envValue); err == nil {
if validator(value) {
return value
}
// Log validation failure
fmt.Fprintf(os.Stderr, "WARNING: Invalid value for %s: %d, using default: %d\n",
envKey, value, defaultValue)
}
}
return defaultValue
}
// Usage
func getDefaultPayloadSizeThreshold() int {
return envutil.GetEnvIntWithValidation(
"MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD",
defaultPayloadSizeThreshold,
func(v int) bool { return v > 0 }, // Validation: must be positive
)
}
Benefits:
- User-visible warnings for invalid configuration
- Flexible validation per configuration value
- Better debugging experience
3. Consider Configuration Struct Pattern
Effort: Medium (4-6 hours)
For larger refactoring, consider moving to struct-based configuration:
type Config struct {
LogDir string `env:"MCP_GATEWAY_LOG_DIR" default:"/tmp/gh-aw/mcp-logs"`
PayloadDir string `env:"MCP_GATEWAY_PAYLOAD_DIR" default:"/tmp/jq-payloads"`
PayloadSizeThreshold int `env:"MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD" default:"1024" validate:"gt=0"`
EnableDIFC bool `env:"MCP_GATEWAY_ENABLE_DIFC" default:"false"`
}
func LoadConfig() (*Config, error) {
// Use struct tags to automatically load from environment
// Consider libraries like github.com/kelseyhightower/envconfig
}
Benefits:
- Centralized configuration definition
- Self-documenting with struct tags
- Easy to add new configuration values
- Type-safe configuration access
Implementation Checklist
Parent Issue
See parent analysis report: #797
Related to #797
AI generated by Duplicate Code Detector
🔍 Duplicate Code Pattern: Flag Environment Variable Getters
Part of duplicate code analysis: #797
Summary
Multiple
getDefault*()functions in theinternal/cmdpackage follow an identical pattern for retrieving configuration values from environment variables with fallback to hardcoded defaults. This pattern is repeated 4 times across flag files with only minor variations.Duplication Details
Pattern: Environment Variable Getter Functions
internal/cmd/flags_logging.go:getDefaultLogDir()(lines 36-41)getDefaultPayloadDir()(lines 45-50)getDefaultPayloadSizeThreshold()(lines 54-64)internal/cmd/flags_difc.go:getDefaultEnableDIFC()(lines 30-38)Duplicated Structure
All functions follow the same pattern:
Code Examples
String Getter (getDefaultLogDir):
String Getter (getDefaultPayloadDir) - Nearly identical:
Integer Getter (getDefaultPayloadSizeThreshold):
Boolean Getter (getDefaultEnableDIFC):
Impact Analysis
Maintainability
Bug Risk
> 0, but what about negative values elsewhere?Code Bloat
Refactoring Recommendations
1. Create Generic Environment Getter Utility
Effort: Low-Medium (2-4 hours)
Extract common pattern into type-safe generic utility:
Usage - Simplifies flag getters to 1 line:
Benefits:
2. Add Validation and Logging
Effort: Low (1-2 hours)
Enhance utility with validation and debug logging:
Benefits:
3. Consider Configuration Struct Pattern
Effort: Medium (4-6 hours)
For larger refactoring, consider moving to struct-based configuration:
Benefits:
Implementation Checklist
internal/envutilpackage with generic gettersmake agent-finishedto verify all checks passParent Issue
See parent analysis report: #797
Related to #797