-
Notifications
You must be signed in to change notification settings - Fork 465
Simultaneous Minimum and Maximum Evaluation #90
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
Changes from 7 commits
655bd5c
3fd03ea
b1453e4
6fd8cc7
49ccd48
cbf35df
0c30dcf
4ee09be
66fd25e
57cbb28
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -383,3 +383,117 @@ extension Collection where Element: Comparable { | |
| return max(count: count, sortedBy: <) | ||
| } | ||
| } | ||
|
|
||
| //===----------------------------------------------------------------------===// | ||
| // Simultaneous minimum and maximum evaluation | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| extension Sequence { | ||
| /// Returns both the minimum and maximum elements in the sequence, using the | ||
| /// given predicate as the comparison between elements. | ||
| /// | ||
| /// The predicate must be a *strict weak ordering* over the elements. That | ||
| /// is, for any elements `a`, `b`, and `c`, the following conditions must | ||
| /// hold: | ||
| /// | ||
| /// - `areInIncreasingOrder(a, a)` is always `false`. (Irreflexivity) | ||
| /// - If `areInIncreasingOrder(a, b)` and `areInIncreasingOrder(b, c)` are | ||
| /// both `true`, then `areInIncreasingOrder(a, c)` is also | ||
| /// `true`. (Transitive comparability) | ||
| /// - Two elements are *incomparable* if neither is ordered before the other | ||
| /// according to the predicate. If `a` and `b` are incomparable, and `b` | ||
| /// and `c` are incomparable, then `a` and `c` are also incomparable. | ||
| /// (Transitive incomparability) | ||
| /// | ||
| /// This example shows how to use the `minAndMax(by:)` method on a | ||
| /// dictionary to find the key-value pair with the lowest value and the pair | ||
| /// with the highest value. | ||
| /// | ||
| /// let hues = ["Heliotrope": 296, "Coral": 16, "Aquamarine": 156] | ||
| /// if let extremeHues = hues.minAndMax(by: {$0.value < $1.value}) { | ||
| /// print(extremeHues.min, extremeHues.max) | ||
| /// } else { | ||
| /// print("There are no hues") | ||
| /// } | ||
| /// // Prints: "(key: "Coral", value: 16) (key: "Heliotrope", value: 296)" | ||
| /// | ||
| /// - Precondition: The sequence is finite. | ||
| /// | ||
| /// - Parameter areInIncreasingOrder: A predicate that returns `true` | ||
| /// if its first argument should be ordered before its second | ||
| /// argument; otherwise, `false`. | ||
| /// - Returns: A tuple with the sequence's minimum element, followed by its | ||
| /// maximum element. For either member, if the sequence provides multiple | ||
| /// qualifying elements, the one chosen is unspecified. The same element may | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I’d like to take a more principled stand than this. Right now we have two different behaviors for which maximum element is chosen. First, /// All instances of `Foo` are equal to each other.
class Foo: Comparable {
static func ==(lhs: Foo, rhs: Foo) -> Bool { true }
static func < (lhs: Foo, rhs: Foo) -> Bool { false }
}
let a = Foo()
let b = Foo()
max(a, b) === b // true
[a, b].sorted().last === b // true
[a, b].max(count: 1).first === b // trueSecond, the sequence version of [a, b].max() === b // falseOn the Even though we’ll end up where this method and the stdlib’s
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Enshrining which element is chosen to break ties seems like an over-specification; someone's code may be buggy if they're depending on a specific equivalent element.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We already specify this in e.g. |
||
| /// be used for both members if all the elements are equivalent. If the | ||
| /// sequence has no elements, returns `nil`. | ||
| /// | ||
| /// - Complexity: O(*n*), where *n* is the length of the sequence. | ||
| public func minAndMax( | ||
| by areInIncreasingOrder: (Element, Element) throws -> Bool | ||
| ) rethrows -> (min: Element, max: Element)? { | ||
| var iterator = makeIterator() | ||
| guard var lowest = iterator.next() else { return nil } | ||
|
|
||
| var highest = iterator.next() ?? lowest | ||
| if try areInIncreasingOrder(highest, lowest) { | ||
| swap(&lowest, &highest) | ||
| } | ||
| while let element = iterator.next() { | ||
| guard let following = iterator.next() else { | ||
| if try areInIncreasingOrder(element, lowest) { | ||
| lowest = element | ||
| } else if try !areInIncreasingOrder(element, highest) { | ||
| highest = element | ||
| } | ||
| break | ||
| } | ||
|
|
||
| if try areInIncreasingOrder(following, element) { | ||
| if try areInIncreasingOrder(following, lowest) { | ||
| lowest = following | ||
| } | ||
| if try !areInIncreasingOrder(element, highest) { | ||
| highest = element | ||
| } | ||
| } else { | ||
| if try areInIncreasingOrder(element, lowest) { | ||
| lowest = element | ||
| } | ||
| if try !areInIncreasingOrder(following, highest) { | ||
| highest = following | ||
| } | ||
| } | ||
| } | ||
| return (lowest, highest) | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The while loop can be substantially simplified, since iterators are guaranteed to keep returning while var low = iterator.next() {
var high = iterator.next() ?? low
if try areInIncreasingOrder(high, low) { swap(&low, &high) }
if try areInIncreasingOrder(low, lowest) { lowest = low }
if try !areInIncreasingOrder(high, highest) { highest = high }
}This does technically perform one extraneous comparison on odd-length sequences ( while var low = iterator.next() {
guard var high = iterator.next() else {
if try areInIncreasingOrder(low, lowest) { lowest = low }
if try !areInIncreasingOrder(low, highest) { highest = low }
break
}
if try areInIncreasingOrder(high, low) { swap(&low, &high) }
if try areInIncreasingOrder(low, lowest) { lowest = low }
if try !areInIncreasingOrder(high, highest) { highest = high }
} |
||
| } | ||
|
|
||
| extension Sequence where Element: Comparable { | ||
| /// Returns both the minimum and maximum elements in the sequence. | ||
| /// | ||
| /// This example finds the smallest and largest values in an array of height | ||
| /// measurements. | ||
| /// | ||
| /// let heights = [67.5, 65.7, 64.3, 61.1, 58.5, 60.3, 64.9] | ||
| /// if let (lowestHeight, greatestHeight) = heights.minAndMax() { | ||
| /// print(lowestHeight, greatestHeight) | ||
| /// } else { | ||
| /// print("The list of heights is empty") | ||
| /// } | ||
| /// // Prints: "58.5 67.5" | ||
| /// | ||
| /// - Precondition: The sequence is finite. | ||
| /// | ||
| /// - Returns: A tuple with the sequence's minimum element, followed by its | ||
| /// maximum element. For either member, if there is a tie for the extreme | ||
| /// value, the element chosen is unspecified. The same element may be used | ||
| /// for both members if all the elements are equal. If the sequence has no | ||
| /// elements, returns `nil`. | ||
| /// | ||
| /// - Complexity: O(*n*), where *n* is the length of the sequence. | ||
| @inlinable | ||
| public func minAndMax() -> (min: Element, max: Element)? { | ||
| return minAndMax(by: <) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: We don't need a return here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like the As a matter of style, I think
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wish we had a style guide for these things. But you're right to be consistent with the whole codebase 😊 (I think). Looks like SE-0250 is not coming back soon, but I could raise it if it does. |
||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Spaces between the curly braces.