Remove all <=> operators and instead use comparator. Fixing #2

Author: danlark1

include/miniselect/private/median_common.h

@@ -270,8 +270,8 @@ template <class Iter, class Compare,
           class DiffType = typename std::iterator_traits<Iter>::difference_type>
 inline DiffType median_index(const Iter r, DiffType a, DiffType b, DiffType c,
                              Compare&& comp) {
-  if (r[a] > r[c]) std::swap(a, c);
-  if (r[b] > r[c]) return c;
+  if (comp(r[c], r[a])) std::swap(a, c);
+  if (comp(r[c], r[b])) return c;
   if (comp(r[b], r[a])) return a;
   return b;
 }
@@ -286,7 +286,6 @@ template <bool leanRight, class Iter, class Compare,
 inline DiffType median_index(Iter r, DiffType a, DiffType b, DiffType c,
                              DiffType d, Compare&& comp) {
   if (comp(r[d], r[c])) std::swap(c, d);
-  assert(r[c] <= r[d]);
   if (leanRight) {
     if (comp(r[c], r[a])) {
       assert(comp(r[c], r[a]) && !comp(r[d], r[c]));  // so r[c]) is out

testing/test_select.cpp

@@ -31,6 +31,13 @@ struct IndirectLess {
   IndirectLess &operator=(IndirectLess &&) = default;
 };
 
+struct CustomInt {
+  size_t x = 0;
+  bool operator<(const CustomInt& other) const {
+    return x < other.x;
+  }
+};
+
 template <typename Selector>
 class SelectTest : public ::testing::Test {
  public:
@@ -135,7 +142,7 @@ class SelectTest : public ::testing::Test {
 
   static void TestCustomComparators() {
     std::vector<std::unique_ptr<int>> v(1000);
-    for (int i = 0; static_cast<std::size_t>(i) < v.size(); ++i) {
+    for (size_t i = 0; i < v.size(); ++i) {
       v[i] = std::make_unique<int>(i);
     }
     Selector::Select(v.begin(), v.begin() + v.size() / 2, v.end(),
@@ -151,6 +158,21 @@ class SelectTest : public ::testing::Test {
     }
   }
 
+  static void TestOnlyOperatorLess() {
+    std::vector<CustomInt> v(1000);
+    for (size_t i = 0; i < v.size(); ++i) {
+      v[i].x = v.size() - i - 1;
+    }
+    Selector::Select(v.begin(), v.begin() + v.size() / 2, v.end());
+    EXPECT_EQ(v[v.size() / 2].x, v.size() / 2);
+    for (size_t i = 0; i < v.size() / 2; ++i) {
+      EXPECT_LE(v[i].x, v.size() / 2);
+    }
+    for (size_t i = v.size() / 2; i < v.size(); ++i) {
+      EXPECT_GE(v[i].x, v.size() / 2);
+    }
+  }
+
   static void TestRepeat(size_t N, size_t M) {
     ASSERT_NE(N, 0);
     ASSERT_GT(N, M);
@@ -239,6 +261,10 @@ TYPED_TEST(SelectTest, TestComparators) {
   TestFixture::TestCustomComparators();
 }
 
+TYPED_TEST(SelectTest, TestOnlyOperatorLess) {
+  TestFixture::TestOnlyOperatorLess();
+}
+
 TYPED_TEST(SelectTest, TestRepeats) { TestFixture::TestManyRepeats(); }
 
 TYPED_TEST(SelectTest, TestRandomAccessIterators) {

testing/test_sort.cpp

@@ -10,6 +10,7 @@
 #include <memory>
 #include <random>
 #include <string>
+#include <tuple>
 #include <vector>
 
 #include "test_common.h"
@@ -31,6 +32,13 @@ struct IndirectLess {
   IndirectLess &operator=(IndirectLess &&) = default;
 };
 
+struct CustomInt {
+  size_t x = 0;
+  bool operator<(const CustomInt& other) const {
+    return x < other.x;
+  }
+};
+
 template <typename Sorter>
 class PartialSortTest : public ::testing::Test {
  public:
@@ -122,15 +130,26 @@ class PartialSortTest : public ::testing::Test {
 
   static void TestCustomComparators() {
     std::vector<std::unique_ptr<int>> v(1000);
-    for (int i = 0; static_cast<std::size_t>(i) < v.size(); ++i) {
+    for (size_t i = 0; i < v.size(); ++i) {
       v[i] = std::make_unique<int>(i);
     }
     Sorter::Sort(v.begin(), v.begin() + v.size() / 2, v.end(), IndirectLess{});
-    for (int i = 0; static_cast<std::size_t>(i) < v.size() / 2; ++i) {
+    for (size_t i = 0; i < v.size() / 2; ++i) {
       ASSERT_NE(v[i], nullptr);
       EXPECT_EQ(*v[i], i);
     }
   }
+
+  static void TestOnlyOperatorLess() {
+    std::vector<CustomInt> v(1000);
+    for (size_t i = 0; i < v.size(); ++i) {
+      v[i].x = v.size() - i - 1;
+    }
+    Sorter::Sort(v.begin(), v.begin() + v.size() / 2, v.end());
+    for (size_t i = 0; i < v.size() / 2; ++i) {
+      EXPECT_EQ(v[i].x, i);
+    }
+  }
 };
 
 TYPED_TEST_SUITE(PartialSortTest, algorithms::All);
@@ -169,6 +188,10 @@ TYPED_TEST(PartialSortTest, TestRandomAccessIterators) {
   TestFixture::TestRandomAccessIterators();
 }
 
+TYPED_TEST(PartialSortTest, TestOnlyOperatorLess) {
+  TestFixture::TestOnlyOperatorLess();
+}
+
 // The standard says that the order of other elements is unspecified even if
 // nothing should be sorted so it fails for libcxx and PDQ which is Ok. Saving
 // this test for a reference.