Confusing output when calling min and max with an empty slice about lo HOT 17 OPEN

Perhaps just panic?

from lo.

hello @tw1nk

I'm not sure to understand your suggestion.

If the slice is empty, what is the minimum/maximum value?

from lo.

The options are:

1. panic
2. Return *T with nil on failure
3. Return (*T, error) with nil+error on failure
4. Return some sentinel value

Of these options, 4 is by far the worst and should not be considered. Code that crashes is much better than code that continues running but gives you the wrong answer.

So I agree panic is better than that. But it's not the best option in my opinion. IMO the best is 2, with 3 being a close second. There is only one meaningful error to return here, so *T offers plenty of information. If it is nil and you deference it, you will get a panic. But you have the option of checking the result first to avoid the panic. This lets the programmer decide how defensive to be, instead of assuming they want the program to crash on invalid input.

What makes 2 better than 3 in my opinion is that if you already know the input is valid, you can write your code using Max in a single line:

if len(input) > 0 {
   myMax = Validate(*lo.Max(input))
}

With 3 you'd have to write:

if len(input) > 0 {
   maxVal, err = lo.Max(input)
   myMax = Validate(*maxVal)
}

And it's unclear if you've really covered all the bases with what err could be, so people are likely to end up writing the even longer version:

if len(input) > 0 {
   maxVal, err = lo.Max(input)
   if err != nil {
     myMax = Validate(*maxVal)
   }
}

That's a lot of code just to get the maximum value out of a function.

from lo.

I believe what @tw1nk suggests is to return the maximum/minimum value for the generic type, e.g. return math.MaxInt when calling Min[int] with an empty slice and returning math.MinInt when calling Max[int] with an empty slice.

I think it has some logic behind it, but there's no non-reflection way of getting the max value of a generic type, and there is a way of getting the zero value

from lo.

I believe what @tw1nk suggests is to return the maximum/minimum value for the generic type, e.g. return math.MaxInt when calling Min[int] with an empty slice and returning math.MinInt when calling Max[int] with an empty slice.

Oh! I see.

Min uses the type constraint constraints.Ordered. There is no easy way to infer a NaN value for string.

I was thinking about a MinOrElse([]int{1, 2, 3}, math.NaN) function. But in that case, you might as well do a len(slice) == 0 condition.

WDYT ?

from lo.

Perhaps just panic?

I see dislikes but let me explain 😄

Applying Min or Max operation to empty slice doesn't make any sense. Take zero division for instance: a / b where b is 0 would panic in Go - you as developer need to ensure that b is non-zero.
Same applicable here: there's no definition for minimal or maximum value of empty slice so this should count is abnormal situation and developer need to ensure that this abnormal situation is prevented in the code.

from lo.

so confused about how to return the min/max value when a slice is empty.

from lo.

Maybe math.NaN() would be better?

from lo.

Maybe math.NaN() would be better?

That’s actually a lot better.

from lo.

I believe since go-lang uses default values according to data type (0 for integer), it only makes sense to "assume" empty slices as zero-valued slices, returning 0 for both min and max operations.

Returning math.NaN() would also force users to always check against presence of math.NaN() thus making usage syntax a little more ugly.

from lo.

Oh! I see.

Min uses the type constraint constraints.Ordered. There is no easy way to infer a NaN value for string.

I was thinking about a MinOrElse([]int{1, 2, 3}, math.NaN) function. But in that case, you might as well do a len(slice) == 0 condition.

WDYT ?

I actually think a MinOrElse function would be valuable as it would have the code appear more streamable and friendly. Would be nice to be able avoid writing a len(slice)==0 condition

from lo.

maybe have it return an error on empty slice since it's more or less impossible to infer the min / max values.

It's also undefined what a min / max value of a string is as @samber points out and that can only be inferred from from comparing more than 0 values.

if only 1 value in the slice the value is both the min and max value.

from lo.

But also adding a MinOrElse and MaxOrElse would probably be a good idea if you don't want to check for errors.

from lo.

I wonder if we could write another helper, using sort.Interface. 🤔

from lo.

Maybe return an error while input is an empty slice🤣

from lo.

Returning 0 or "default" on empty slice would be disastrous IMO. For many datasets 0 is an enormously large or small number. It should return nil or an error. nil is probably much better since you can then still use these functions inline and just dereference the output if you are sure the input is safe. With a separate error you'd have to call min/max in separate line.

from lo.

5. What if we use the same semantic as go-channels do?
   If the empty slice is given, return a default value + ok value (true if returned value is valid).

But the bad thing is, that the lo.Max() function signature and behavior are already set. And if you change the signature, you will lead clients of the package to suffer if they upgrade :(

from lo.