Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mathutil.Max bug #292

Open
feeops opened this issue Feb 8, 2025 · 2 comments
Open

mathutil.Max bug #292

feeops opened this issue Feb 8, 2025 · 2 comments

Comments

@feeops
Copy link

feeops commented Feb 8, 2025

// Max return max value of numbers.
// Play: https://go.dev/play/p/cN8DHI0rTkH
func Max[T constraints.Integer | constraints.Float](numbers ...T) T {
	max := numbers[0]

	for _, v := range numbers {
		if max < v {
			max = v
		}
	}

	return max
}

if numbers is empty,it will panic

@cannian1
Copy link
Collaborator

cannian1 commented Feb 9, 2025

If we follow the function signature of the standard library 1.21+, func max[T cmp.Ordered](x T, y ...T) T
this will break backward compatibility.

Should we change it this way? Or would it be more elegant to throw a panic when the number of arguments passed is zero? @duke-git

@duke-git
Copy link
Owner

duke-git commented Feb 10, 2025

If we follow the function signature of the standard library 1.21+, func max[T cmp.Ordered](x T, y ...T) T this will break backward compatibility.

Should we change it this way? Or would it be more elegant to throw a panic when the number of arguments passed is zero? @duke-git

@cannian1, There is no more elegant way to solve this problem, we can choose one of the following 4 methods. Each one has its drawbacks. I prefer the option 4 (last one).

  1. Return the zero value, when the number of arguments passed is zero. (drawback: this can't tell the difference of params case like: []int{}, []int{0,0,0})
func Max[T constraints.Integer | constraints.Float](numbers ...T) T {
	if len(numbers) == 0 {
		var zero T
		return zero
	}

	max := numbers[0]

	for _, v := range numbers {
		if max < v {
			max = v
		}
	}

	return max
}
  1. Follow the function signature of the standard library 1.21+, func max[T cmp.Ordered](x T, y ...T) T, this will break backward compatibility.

  2. Return an error if the slice is empty. this will also break backward compatibility.

func Max[T constraints.Integer | constraints.Float](numbers ...T) (T, error) {
    if len(numbers) == 0 {
        var zero T
        return zero, fmt.Errorf("no numbers")
    }

    max := numbers[0]
    for _, v := range numbers {
        if max < v {
            max = v
        }
    }

    return max, nil
}
  1. Throw a panic info when the number of arguments passed is zero. This is also the way of slices std package. (slices.Max Although its param type is slice)
func Max[T constraints.Integer | constraints.Float](numbers ...T) T {
	if len(numbers) == 0 {
		panic("mathutil.Max: empty numbers")
	}

	max := numbers[0]

	for _, v := range numbers {
		if max < v {
			max = v
		}
	}

	return max
}

duke-git added a commit that referenced this issue Feb 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants