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

[stdlib] Make comparison operator choices consistent #137

Closed
wants to merge 1 commit into from

Conversation

T-Pham
Copy link

@T-Pham T-Pham commented Dec 4, 2015

Since all other comparisons in the min and max functions use the less-than operator, this line should use the same operator as well.

Since all other comparisons in the `min` and `max` functions use the less-than operator, this line should use the same operator as well.
@gribozavr gribozavr self-assigned this Dec 4, 2015
@lilyball
Copy link
Contributor

lilyball commented Dec 4, 2015

This actually changes the (undocumented) semantics of the function slightly. If you have multiple distinct values that nevertheless compare as equal, the old implementation will return the last value in nearly all cases*, but your new implementation will return the first value (unless the first and second arguments are equal, then it will return the second value).

*The one case it won't is if you pass 3 or more values, where the 2nd and 3rd argument compare equal (and are the maximal of the arguments), then it will return the 2nd argument instead of the 3rd.

@gribozavr
Copy link
Contributor

Right. I remember someone making an argument that min and max should satisfy certain axioms, and that determines which value should be returned among equal ones -- the first one or the last one. I don't remember the argument of top of my head, though.

@T-Pham
Copy link
Author

T-Pham commented Dec 4, 2015

I see the point. This raises two questions:

  • Should we make the case @kballard mentioned in the * section consistent with other cases? That is to return the 3rd argument when the 2nd and 3rd argument compare equal.
  • Should there be a universal rule on this issue for similar situations of the whole project? For instance, I see that the NSArray's indexOfObjectPassingTest(_:) method returns the lowest passing index. This is somehow inconsistent with this min and max methods, which return that last value, IMO.

@lilyball
Copy link
Contributor

lilyball commented Dec 4, 2015

I think we should definitely fix the odd behavior here so min() and max() always return either the first or the last such value (and that they behave the same). However, the behavior of indexOfObjectPassingTest(_:) is unrelated; it's expected that that method uses short-circuiting behavior, such that it stops testing objects as soon as it finds a match, and principle of least surprise suggests that people will expect it to start from the first value in the array. Whereas min() and max() must test every element, so there's no short-circuiting available and thus no real performance difference*.

So this basically boils down to two considerations:

  1. Does anybody currently rely on max() (and probably min() but I haven't looked at that code) actually returning the last equivalent value? Probably not, especially because there's that one specific case where it doesn't actually do that, but it's worth considering.
  2. Ignoring the current behavior entirely, is it less surprising to return the first maximal element or the last maximal element? This is hard to answer, because it's very subjective. Personally, my vague impression is that max() "feels like" it should return the last one, and min() "feels like" it should return the first one, but being inconsistent is a bad idea so I think vague impressions of what feels better is pretty irrelevant. Given that, I'm inclined to say we should just go with the first maximal element because it's pretty much an arbitrary decision.

*There may be a slight benefit to starting from the beginning of the array instead of the end, as it's usually faster to iterate forward over memory than backwards, but that's probably negligible here.

@gribozavr
Copy link
Contributor

Ignoring the current behavior entirely, is it less surprising to return the first maximal element or the last maximal element? This is hard to answer, because it's very subjective.

It is not subjective. As I said, there are interesting non-trivial mathematical properties (IIRC, noted by Stepanov) that justify it one way (and possibly differently for min and max, I don't remember). It would be good if someone did more research on that.

@lilyball
Copy link
Contributor

lilyball commented Dec 4, 2015

Here's a list of papers by Alexander Stepanov. I skimmed through Fundamentals of Generic Programming (PDF), which describes the desire to have axioms and "concepts", and gives a couple of examples but does not include anything about max. I'm not sure how to find a paper that actually describes any relevant axioms here, since the list of papers is quite long. However, he does mention the STL, so I figured I'd see how the STL defines max:

Return value
1-2) The greater of a and b. If they are equivalent, returns a.
3-4) The greatest value in ilist. If several values are equivalent to the greatest, returns the leftmost one.

min also has the same leftmost rules.

It seems to me that following the STL's behavior here seems like a pretty good choice. If you can find a Stepanov paper that actually describes a relevant axiom, I'd be interested in seeing that, but I suspect that if he does describe one, it probably results in picking the leftmost one.

@gribozavr
Copy link
Contributor

@lilyball
Copy link
Contributor

lilyball commented Dec 4, 2015

Huh, so that actually does match my "feels like" intuition. More importantly, it throws out the "they should behave the same" argument, saying that they should definitely not. The following argument does seem pretty compelling though:

  • the pair (min(x,y), max(x,y)) is either (x,y) or (y,x)

I'm convinced. We should adopt this behavior.

@T-Pham
Copy link
Author

T-Pham commented Dec 4, 2015

+1 for adopting the behavior.
One more consideration is if this behavior should be explicitly stated in the documentation so that users can be guaranteed to rely on it.

@dabrahams
Copy link
Contributor

[AFK] When used in sorting algorithms:

(x,y) = (min(x,y), max(x,y))

Should preserve identity and order if x and y are unordered w.r.t. one another

@gribozavr
Copy link
Contributor

#566 adds tests that show that we already implement that.

@gribozavr gribozavr closed this Dec 16, 2015
dabelknap added a commit to dabelknap/swift that referenced this pull request Nov 19, 2018
maldahleh pushed a commit to maldahleh/swift that referenced this pull request Oct 26, 2020
freak4pc pushed a commit to freak4pc/swift that referenced this pull request Sep 28, 2022
Update Swift 4.0 compatibility support for `IBAnimatable` project.
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

Successfully merging this pull request may close these issues.

4 participants