Skip to content

Conversation

@m16vie
Copy link

@m16vie m16vie commented Dec 4, 2015

No description provided.

@gribozavr
Copy link
Contributor

Does this change implement what was discussed in pull request #137?

@colinta
Copy link

colinta commented Dec 4, 2015

This changes the behavior of max. If a class implements Comparable, calling max(a, b, c) currently returns the last instance that has the maximum value. This means that if a > b, b == c, the current version of max returns c, but your version returns b.

Neither is correct or wrong, but the change is unnecessary.

@m16vie
Copy link
Author

m16vie commented Dec 4, 2015

Also, currently max(a, b, c) returns b assuming a < b, b == c and max(a, b, c, d) returns d assuming a < b, b == c == d, which seems to be wrong. Following the argument that given equal parameters, min and max should return different instances, max should then return the last maximum instance while min returns the first minimum. I suggest the following code.

/// Returns the greatest argument passed.
@warn_unused_result
public func max<T : Comparable>(x: T, _ y: T, _ z: T, _ rest: T...) -> T {
  var r = x
  if y >= r {
    r = y
  }
  if z >= r {
    r = z
  }
  for t in rest {
    if t >= r {
      r = t
    }
  }
  return r
}

@gribozavr
Copy link
Contributor

@mmkg Please update the pull request and provide tests.

@IngCr3at1on
Copy link

@colinta I think the general idea was to create more consistency in the language handling in the first place (both are "valid" but currently they are inconsistent to my understanding). This was discussed in great detail on #137

@gribozavr
Copy link
Contributor

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

@gribozavr gribozavr closed this Dec 16, 2015
dabelknap added a commit to dabelknap/swift that referenced this pull request Jan 16, 2019
…member-types

Ensure a space inside braces in a single member type decl.
maldahleh pushed a commit to maldahleh/swift that referenced this pull request Oct 26, 2020
[AutoDiff upstream] Update gyb-generated files.
freak4pc pushed a commit to freak4pc/swift that referenced this pull request Sep 28, 2022
XFAIL CoreStore for 3.2, 4.0 on {tv, watch}OS due to SDK changes
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