-
Notifications
You must be signed in to change notification settings - Fork 15
Closed
Description
Part of duplicate code analysis: #1006
Summary
High-impact duplication in internal/config/config_stdin.go where optional pointer field assignment is repeated 3 times in a single function. This repetitive pattern reduces code clarity and makes future config field additions error-prone.
Duplication Details
Pattern: Repeated Pointer Nil-Check and Dereference
- Severity: HIGH
- Occurrences: 3 instances in one function (15 lines total)
- Locations:
internal/config/config_stdin.golines 145-156 (convertStdinConfigfunction)
Code Sample (lines 145-156):
// Apply optional port override
if stdinCfg.Gateway.Port != nil {
cfg.Gateway.Port = *stdinCfg.Gateway.Port
}
// Apply optional timeout overrides
if stdinCfg.Gateway.StartupTimeout != nil {
cfg.Gateway.StartupTimeout = *stdinCfg.Gateway.StartupTimeout
}
if stdinCfg.Gateway.ToolTimeout != nil {
cfg.Gateway.ToolTimeout = *stdinCfg.Gateway.ToolTimeout
}Each field requires:
- Nil check on optional pointer
- Dereferencing and assignment to target config
- Identical error-prone structure
Impact Analysis
- Maintainability: HIGH - Adding new optional fields requires copy-pasting this pattern
- Bug Risk: MEDIUM - Easy to forget nil check or dereference incorrectly
- Code Clarity: MEDIUM - Repetitive pattern obscures the actual configuration logic
- Future Risk: As more optional fields are added, this pattern will proliferate
Refactoring Recommendations
1. Extract Generic Optional Field Helper
Recommended implementation:
// Add to internal/config/config_stdin.go or internal/config/helpers.go
// applyOptionalInt applies a non-nil optional integer value to a destination.
// If source is nil, destination remains unchanged.
func applyOptionalInt(source *int, destination *int) {
if source != nil {
*destination = *source
}
}
// For string fields, if needed:
func applyOptionalString(source *string, destination *string) {
if source != nil {
*destination = *source
}
}
// Generic version using type parameters (Go 1.18+):
func applyOptional[T any](source *T, destination *T) {
if source != nil {
*destination = *source
}
}Refactored usage in convertStdinConfig (lines 145-156):
// Apply optional overrides using helper
applyOptionalInt(stdinCfg.Gateway.Port, &cfg.Gateway.Port)
applyOptionalInt(stdinCfg.Gateway.StartupTimeout, &cfg.Gateway.StartupTimeout)
applyOptionalInt(stdinCfg.Gateway.ToolTimeout, &cfg.Gateway.ToolTimeout)- Estimated effort: 1-2 hours (create helper, refactor callers, test)
- Benefits:
- Reduces 15 lines to 3 lines (80% reduction)
- Single point of change for optional field logic
- Type-safe with generics
- Self-documenting code
- Easy to extend for new optional fields
2. Alternative: Use Struct Helper Method
// Method on StdinGatewayConfig
func (s *StdinGatewayConfig) applyOverrides(cfg *GatewayConfig) {
if s.Port != nil {
cfg.Port = *s.Port
}
if s.StartupTimeout != nil {
cfg.StartupTimeout = *s.StartupTimeout
}
if s.ToolTimeout != nil {
cfg.ToolTimeout = *s.ToolTimeout
}
}
// Usage
stdinCfg.Gateway.applyOverrides(cfg.Gateway)This encapsulates the logic but is less generic than option #1.
Implementation Checklist
- Review approach with team (generic helper vs struct method)
- Implement chosen helper function
- Refactor
convertStdinConfigto use helper - Verify all config parsing tests pass
- Test with JSON stdin config containing optional fields
- Test with JSON stdin config with all fields nil
- Document helper function usage
- Consider applying pattern to other optional fields in codebase
Parent Issue
See parent analysis report: #1006
Related to #1006
Generated by Duplicate Code Detector
- expires on Feb 24, 2026, 3:01 AM UTC
Reactions are currently unavailable
Metadata
Metadata
Assignees
Type
Fields
Give feedbackNo fields configured for issues without a type.