Refactor duplicate array processing logic into helper functions #5

New Issue

b3nw · 2025-07-11T15:08:46-05:00

b3nw commented

2025-07-11 15:08:46 -05:00

Refactoring: Code Duplication in Label ID Processing

Issue Description

The label module contains duplicate logic for processing and validating label ID arrays across multiple functions, leading to code duplication and maintenance overhead.

Affected Functions

AddIssueLabelsFn (lines ~345-357)
RemoveIssueLabelsFn (lines ~385-397)
ReplaceIssueLabelsFn (lines ~435-447)

Current Duplicate Code Pattern

labelIDs := make([]int64, len(labelsData))
for i, labelData := range labelsData {
    if labelID, ok := labelData.(float64); ok {
        labelIDs[i] = int64(labelID)
    } else {
        return to.ErrorResult(fmt.Errorf("invalid label ID at index %d", i))
    }
}

Problems

Code Duplication: Same logic repeated 3 times
Missing Validation: No check for negative numbers
Maintenance Overhead: Changes need to be made in multiple places
Inconsistent Error Messages: Slight variations in error handling

Proposed Solution

Extract Helper Function

// validateAndConvertLabelIDs converts and validates an array of label IDs
func validateAndConvertLabelIDs(labelsData []interface{}) ([]int64, error) {
    if len(labelsData) == 0 {
        return nil, fmt.Errorf("label IDs array cannot be empty")
    }
    
    if len(labelsData) > 100 { // Prevent API abuse
        return nil, fmt.Errorf("cannot process more than 100 labels at once")
    }
    
    labelIDs := make([]int64, len(labelsData))
    for i, labelData := range labelsData {
        if labelID, ok := labelData.(float64); ok && labelID > 0 {
            labelIDs[i] = int64(labelID)
        } else {
            return nil, fmt.Errorf("invalid label ID at index %d: must be a positive number", i)
        }
    }
    return labelIDs, nil
}

Usage in Functions

// In AddIssueLabelsFn, RemoveIssueLabelsFn, ReplaceIssueLabelsFn
labelIDs, err := validateAndConvertLabelIDs(labelsData)
if err != nil {
    return to.ErrorResult(err)
}

Benefits

Reduced Duplication: Single source of truth for label ID processing
Better Validation: Includes positive number check and array size limits
Consistent Error Messages: Standardized error handling
Easier Maintenance: Changes only need to be made in one place
Improved Testing: Can unit test the helper function independently

Implementation Location

Add the helper function to /operation/label/label.go before the existing functions.

Priority

Medium - Code quality improvement

This refactoring would also address the validation issues mentioned in issue #4

# Refactoring: Code Duplication in Label ID Processing ## Issue Description The label module contains duplicate logic for processing and validating label ID arrays across multiple functions, leading to code duplication and maintenance overhead. ## Affected Functions 1. `AddIssueLabelsFn` (lines ~345-357) 2. `RemoveIssueLabelsFn` (lines ~385-397) 3. `ReplaceIssueLabelsFn` (lines ~435-447) ## Current Duplicate Code Pattern ```go labelIDs := make([]int64, len(labelsData)) for i, labelData := range labelsData { if labelID, ok := labelData.(float64); ok { labelIDs[i] = int64(labelID) } else { return to.ErrorResult(fmt.Errorf("invalid label ID at index %d", i)) } } ``` ## Problems 1. **Code Duplication**: Same logic repeated 3 times 2. **Missing Validation**: No check for negative numbers 3. **Maintenance Overhead**: Changes need to be made in multiple places 4. **Inconsistent Error Messages**: Slight variations in error handling ## Proposed Solution ### Extract Helper Function ```go // validateAndConvertLabelIDs converts and validates an array of label IDs func validateAndConvertLabelIDs(labelsData []interface{}) ([]int64, error) { if len(labelsData) == 0 { return nil, fmt.Errorf("label IDs array cannot be empty") } if len(labelsData) > 100 { // Prevent API abuse return nil, fmt.Errorf("cannot process more than 100 labels at once") } labelIDs := make([]int64, len(labelsData)) for i, labelData := range labelsData { if labelID, ok := labelData.(float64); ok && labelID > 0 { labelIDs[i] = int64(labelID) } else { return nil, fmt.Errorf("invalid label ID at index %d: must be a positive number", i) } } return labelIDs, nil } ``` ### Usage in Functions ```go // In AddIssueLabelsFn, RemoveIssueLabelsFn, ReplaceIssueLabelsFn labelIDs, err := validateAndConvertLabelIDs(labelsData) if err != nil { return to.ErrorResult(err) } ``` ## Benefits 1. **Reduced Duplication**: Single source of truth for label ID processing 2. **Better Validation**: Includes positive number check and array size limits 3. **Consistent Error Messages**: Standardized error handling 4. **Easier Maintenance**: Changes only need to be made in one place 5. **Improved Testing**: Can unit test the helper function independently ## Implementation Location Add the helper function to `/operation/label/label.go` before the existing functions. ## Priority **Medium** - Code quality improvement ## Related Issues - This refactoring would also address the validation issues mentioned in issue #4

b3nw added the enhancement label-management labels 2025-07-11 15:08:53 -05:00

b3nw closed this issue

2025-07-11 15:27:46 -05:00

This repo is archived. You cannot comment on issues.

1 Participants

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: b3nw/gitea-mcp#5