Minor revisions to minAndMax() (#143)

* Settle on 2-way approach (s/o to @NevinBR)
* Add tests for predicate usage and stability
* Update the return value semantics

Author: natecook1000

Sources/Algorithms/MinMax.swift

@@ -423,10 +423,10 @@ extension Sequence {
   ///   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
-  ///   be used for both members if all the elements are equivalent. If the
-  ///   sequence has no elements, returns `nil`.
+  ///   maximum element. If the sequence provides multiple qualifying minimum
+  ///   elements, the first equivalent element is returned; of multiple maximum
+  ///   elements, the last is returned. If the sequence has no elements, the
+  ///   method returns `nil`.
   ///
   /// - Complexity: O(*n*), where *n* is the length of the sequence.
   public func minAndMax(
@@ -440,55 +440,16 @@ extension Sequence {
     // Confirm the initial bounds.
     if try areInIncreasingOrder(highest, lowest) { swap(&lowest, &highest) }
 
-    #if true
     // Read the elements in pairwise.  Structuring the comparisons around this
     // is actually faster than loops based on extracting and testing elements
     // one-at-a-time.
     while var low = iterator.next() {
-      if var high = iterator.next() {
-        // Update the upper bound with the larger new element.
-        if try areInIncreasingOrder(high, low) { swap(&low, &high) }
-        if try !areInIncreasingOrder(high, highest) { highest = high }
-      } else {
-        // Update the upper bound by reusing the last element.  The next element
-        // iteration will also fail, ending the loop.
-        if try !areInIncreasingOrder(low, highest) { highest = low }
-      }
-
-      // Update the lower bound with the smaller new element, which may need a
-      // swap first to determine.
-      if try areInIncreasingOrder(low, lowest) { lowest = low }
-    }
-    #else
-    /// Ensure the second argument has a value that is ranked at least as much as
-    /// the first argument.
-    func sort(_ a: inout Element, _ b: inout Element) throws {
-      if try areInIncreasingOrder(b, a) { swap(&a, &b) }
-    }
-
-    /// Find the smallest and largest values out of a group of four arguments.
-    func minAndMaxOf4(
-      _ a: Element, _ b: Element, _ c: Element, _ d: Element
-    ) throws -> (min: Element, max: Element) {
-      var (a, b, c, d) = (a, b, c, d)
-      try sort(&a, &b)
-      try sort(&c, &d)
-      try sort(&a, &c)
-      try sort(&b, &d)
-      return (a, d)
-    }
-
-    // Read the elements in four-at-a-time.  Some say this is more effective
-    // than a two-at-a-time loop.
-    while let a = iterator.next() {
-      let b = iterator.next() ?? a
-      let c = iterator.next() ?? b
-      let d = iterator.next() ?? c
-      let (low, high) = try minAndMaxOf4(a, b, c, d)
+      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 }
     }
-    #endif
+
     return (lowest, highest)
   }
 }
@@ -510,10 +471,10 @@ extension Sequence where Element: Comparable {
   /// - 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`.
+  ///   maximum element. If the sequence provides multiple qualifying minimum
+  ///   elements, the first equivalent element is returned; of multiple maximum
+  ///   elements, the last is returned. If the sequence has no elements, the
+  ///   method returns `nil`.
   ///
   /// - Complexity: O(*n*), where *n* is the length of the sequence.
   @inlinable

Tests/SwiftAlgorithmsTests/MinMaxTests.swift

@@ -248,4 +248,27 @@ final class MinAndMaxTests: XCTestCase {
     XCTAssertEqual(result2?.min, "a")
     XCTAssertEqual(result2?.max, "g")
   }
+
+  /// Confirms that the given predicate is used to find the min/max.
+  func testPredicate() {
+    let result = (1...5).minAndMax(by: >)
+    XCTAssertEqual(result?.min, 5)
+    XCTAssertEqual(result?.max, 1)
+
+    // Odd count
+    let result2 = "gfabdec".minAndMax(by: >)
+    XCTAssertEqual(result2?.min, "g")
+    XCTAssertEqual(result2?.max, "a")
+  }
+
+  /// Confirms that the min and max are "stable" as defined for minAndMax.
+  func testStability() {
+    for n in 1...10 {
+      let result = repeatElement(0, count: n)
+        .enumerated()
+        .minAndMax(by: { $0.element < $1.element })
+      XCTAssertEqual(result?.min.offset, 0)
+      XCTAssertEqual(result?.max.offset, n - 1)
+    }
+  }
 }