π Duplicate Code Pattern: TimeoutPositive and PositiveInteger Overlapping Validators
Part of duplicate code analysis: #7078
Summary
internal/config/rules/rules.go contains two functions β TimeoutPositive and PositiveInteger β that implement identical validation logic (check value >= 1) with nearly identical structure. TimeoutPositive is already a special case of the existing TimeoutMinimum function (with min=1), and its body is a near-copy of PositiveInteger.
Duplication Details
Pattern: Two functions that both validate value >= 1 with the same structure
- Severity: LowβMedium
- Occurrences: 2 functions in
rules.go (lines 109β121 and 125β137)
- Locations:
internal/config/rules/rules.go:109 β TimeoutPositive
internal/config/rules/rules.go:125 β PositiveInteger
TimeoutPositive (lines 109β121):
func TimeoutPositive(timeout int, fieldName, jsonPath string) *ValidationError {
log.Printf("Validating timeout: field=%s, value=%d, jsonPath=%s", fieldName, timeout, jsonPath)
if timeout < 1 {
log.Printf("Timeout validation failed: %s=%d is not positive", fieldName, timeout)
return &ValidationError{
Field: fieldName,
Message: fmt.Sprintf("%s must be at least 1, got %d", fieldName, timeout),
JSONPath: jsonPath,
Suggestion: "Use a positive number of seconds (e.g., 30)",
}
}
return nil
}
PositiveInteger (lines 125β137):
func PositiveInteger(value int, fieldName, jsonPath string) *ValidationError {
log.Printf("Validating positive integer: field=%s, value=%d, jsonPath=%s", fieldName, value, jsonPath)
if value < 1 {
log.Printf("Positive integer validation failed: %s=%d is not positive", fieldName, value)
return &ValidationError{
Field: fieldName,
Message: fmt.Sprintf("%s must be a positive integer (>= 1), got %d", fieldName, value),
JSONPath: jsonPath,
Suggestion: fmt.Sprintf("Use a positive integer (>= 1) for %s", fieldName),
}
}
return nil
}
The conditional (< 1), structure, logging pattern, and ValidationError construction are all identical. Only the log message strings and ValidationError.Message/Suggestion differ.
Further, TimeoutPositive is structurally equivalent to TimeoutMinimum(timeout, 1, fieldName, jsonPath) β an already-existing function in the same file.
Impact Analysis
- Maintainability: Adding a new field to
ValidationError (e.g., Severity) requires updating both functions identically.
- Bug Risk: Low β these are simple validators in a single file. However, the naming (
TimeoutPositive vs PositiveInteger) obscures that they do the same thing.
- Code Bloat: ~13 lines Γ 2 = 26 lines for essentially one concept.
Refactoring Recommendations
-
Simplest: implement TimeoutPositive as a thin wrapper over TimeoutMinimum
// TimeoutPositive validates that a timeout value is at least 1.
// It is a convenience wrapper around TimeoutMinimum with min=1.
func TimeoutPositive(timeout int, fieldName, jsonPath string) *ValidationError {
return TimeoutMinimum(timeout, 1, fieldName, jsonPath)
}
This removes ~10 lines while preserving the existing public API.
-
Alternative: unify into PositiveInteger and deprecate TimeoutPositive
Since TimeoutPositive is only called once (validation.go:391 for startupTimeout), it could be replaced by PositiveInteger at the call site and removed entirely.
- Call site change:
rules.TimeoutPositive(*gateway.StartupTimeout, ...) β rules.PositiveInteger(*gateway.StartupTimeout, ...)
- Then delete
TimeoutPositive from rules.go.
- Estimated effort: 15β30 minutes
Implementation Checklist
Parent Issue
See parent analysis report: #7078
Related to #7078
Generated by Duplicate Code Detector Β· sonnet46 6.1M Β· β·
π Duplicate Code Pattern:
TimeoutPositiveandPositiveIntegerOverlapping ValidatorsPart of duplicate code analysis: #7078
Summary
internal/config/rules/rules.gocontains two functions βTimeoutPositiveandPositiveIntegerβ that implement identical validation logic (check value >= 1) with nearly identical structure.TimeoutPositiveis already a special case of the existingTimeoutMinimumfunction (withmin=1), and its body is a near-copy ofPositiveInteger.Duplication Details
Pattern: Two functions that both validate
value >= 1with the same structurerules.go(lines 109β121 and 125β137)internal/config/rules/rules.go:109βTimeoutPositiveinternal/config/rules/rules.go:125βPositiveIntegerTimeoutPositive (lines 109β121):
PositiveInteger (lines 125β137):
The conditional (
< 1), structure, logging pattern, andValidationErrorconstruction are all identical. Only the log message strings andValidationError.Message/Suggestiondiffer.Further,
TimeoutPositiveis structurally equivalent toTimeoutMinimum(timeout, 1, fieldName, jsonPath)β an already-existing function in the same file.Impact Analysis
ValidationError(e.g.,Severity) requires updating both functions identically.TimeoutPositivevsPositiveInteger) obscures that they do the same thing.Refactoring Recommendations
Simplest: implement
TimeoutPositiveas a thin wrapper overTimeoutMinimumThis removes ~10 lines while preserving the existing public API.
Alternative: unify into
PositiveIntegerand deprecateTimeoutPositiveSince
TimeoutPositiveis only called once (validation.go:391forstartupTimeout), it could be replaced byPositiveIntegerat the call site and removed entirely.rules.TimeoutPositive(*gateway.StartupTimeout, ...)βrules.PositiveInteger(*gateway.StartupTimeout, ...)TimeoutPositivefromrules.go.Implementation Checklist
internal/config/rules/rules.gointernal/config/validation.goif deletingTimeoutPositivemake testto verify no regressionParent Issue
See parent analysis report: #7078
Related to #7078