Skip to content

Commit

Permalink
Remove all <=> operators and instead use comparator. Fixing #2
Browse files Browse the repository at this point in the history
  • Loading branch information
danlark1 committed Jan 23, 2021
1 parent 5203ccf commit 44d09dc
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 6 deletions.
5 changes: 2 additions & 3 deletions include/miniselect/private/median_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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
Expand Down
28 changes: 27 additions & 1 deletion testing/test_select.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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(),
Expand All @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
27 changes: 25 additions & 2 deletions testing/test_sort.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <memory>
#include <random>
#include <string>
#include <tuple>
#include <vector>

#include "test_common.h"
Expand All @@ -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:
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 44d09dc

Please sign in to comment.