Skip to content

Commit 5c40f6f

Browse files
committed
Bug fix + improvements to probe::exc
When a cycle started and ended with elements comparing equal, probe::exc returned an incorrect result, which this commit fixes. Additionally, this commit fixes the Exc(X) + 1 <= Ham(X) relation when X is already sorted, and improves the space complexity of probe::exc by getting rid of the 'sorted' vector in a similar way that what was recently done for indirect_adapter.
1 parent a4ae870 commit 5c40f6f

File tree

3 files changed

+63
-28
lines changed

3 files changed

+63
-28
lines changed

include/cpp-sort/probes/exc.h

Lines changed: 50 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2016-2022 Morwenn
2+
* Copyright (c) 2016-2025 Morwenn
33
* SPDX-License-Identifier: MIT
44
*/
55
#ifndef CPPSORT_PROBES_EXC_H_
@@ -11,7 +11,6 @@
1111
#include <functional>
1212
#include <iterator>
1313
#include <utility>
14-
#include <vector>
1514
#include <cpp-sort/sorter_facade.h>
1615
#include <cpp-sort/sorter_traits.h>
1716
#include <cpp-sort/utility/as_function.h>
@@ -29,6 +28,15 @@ namespace probe
2928
{
3029
namespace detail
3130
{
31+
template<typename ForwardIterator, typename Compare, typename Projection>
32+
constexpr auto compare_equivalent(ForwardIterator it1, ForwardIterator it2,
33+
Compare compare, Projection projection)
34+
-> bool
35+
{
36+
return not compare(projection(*it1), projection(*it2))
37+
&& not compare(projection(*it2), projection(*it1));
38+
}
39+
3240
template<typename ForwardIterator, typename Compare, typename Projection>
3341
auto exc_probe_algo(ForwardIterator first, ForwardIterator last,
3442
cppsort::detail::difference_type_t<ForwardIterator> size,
@@ -61,48 +69,65 @@ namespace probe
6169
////////////////////////////////////////////////////////////
6270
// Count the number of cycles
6371

64-
std::vector<bool> sorted(size, false);
65-
6672
// Element where the current cycle starts
6773
auto start = first;
6874

6975
difference_type cycles = 0;
70-
while (start != last) {
76+
do {
7177
// Find the element to put in current's place
7278
auto current = start;
7379
auto next_pos = std::distance(first, current);
74-
auto next = iterators[next_pos];
75-
sorted[next_pos] = true;
80+
// We replace all "sorted" iterators with last to make it
81+
// possible to find unsorted iterators between cycles
82+
auto next = std::exchange(iterators[next_pos], last);
7683

7784
// Process the current cycle
78-
if (next != current) {
85+
if (next == current) {
86+
// An element already in its final position counts as one cycle
87+
++cycles;
88+
} else {
89+
bool all_equivalent = true;
90+
91+
// If an element is in the place of another element that compares
92+
// equivalent, it means that this element was actually already in
93+
// a suitable place, so we count one more cycle as if it was an
94+
// already suitably placed element, this handles collections with
95+
// several elements which compare equivalent
96+
if (compare_equivalent(current, next, comp, proj)) {
97+
++cycles;
98+
} else {
99+
all_equivalent = false;
100+
}
101+
79102
while (next != start) {
80-
// If an element is in the place of another element that compares
81-
// equivalent, it means that this element was actually already in
82-
// a suitable place, so we count one more cycle as if it was an
83-
// already suitably placed element, this handles collections with
84-
// several elements which compare equivalent
85-
if (not comp(proj(*next), proj(*current)) &&
86-
not comp(proj(*current), proj(*next))) {
87-
++cycles;
88-
}
89103
// Locate the next element of the cycle
90104
current = next;
91105
auto next_pos = std::distance(first, next);
92-
next = iterators[next_pos];
93-
sorted[next_pos] = true;
106+
next = std::exchange(iterators[next_pos], last);
107+
108+
if (compare_equivalent(current, next, comp, proj)) {
109+
++cycles;
110+
} else {
111+
all_equivalent = false;
112+
}
94113
}
95-
}
96114

97-
++cycles;
115+
// If all elements of a cycle compare equivalent, we already added one
116+
// cycle per element, so we only add one there if at least two elements
117+
// compared different
118+
if (not all_equivalent) {
119+
++cycles;
120+
}
121+
}
98122

99123
// Find the next cycle
100-
auto&& sorted_it = sorted.begin() + std::distance(first, start);
124+
auto&& iterators_it = iterators.begin() + std::distance(first, start);
101125
do {
102126
++start;
103-
++sorted_it;
104-
} while (start != last && *sorted_it);
105-
}
127+
++iterators_it;
128+
} while (start != last && *iterators_it == last);
129+
} while (start != last);
130+
106131
return size - cycles;
107132
}
108133

tests/probes/exc.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2016-2022 Morwenn
2+
* Copyright (c) 2016-2025 Morwenn
33
* SPDX-License-Identifier: MIT
44
*/
55
#include <forward_list>
@@ -41,7 +41,7 @@ TEST_CASE( "presortedness measure: exc", "[probe][exc]" )
4141
CHECK( exc(li.begin(), li.end()) == max_n );
4242
}
4343

44-
SECTION( "regressions" )
44+
SECTION( "regression: ascending duplicates" )
4545
{
4646
std::vector<int> collection;
4747
collection.reserve(100);
@@ -50,4 +50,10 @@ TEST_CASE( "presortedness measure: exc", "[probe][exc]" )
5050

5151
CHECK( exc(collection) == 0 );
5252
}
53+
54+
SECTION( "regression: first and last elements of a cycle compare equal" )
55+
{
56+
std::vector<int> collection = { 0, 0, -1 };
57+
CHECK( exc(collection) == 1 );
58+
}
5359
}

tests/probes/relations.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,11 @@ TEST_CASE( "relations between measures of presortedness", "[probe]" )
4949
// by Ola Petersson and Alistair Moffat
5050
CHECK( runs <= rem + 1 );
5151

52-
CHECK( exc + 1 <= ham );
52+
if (exc == 0) {
53+
CHECK( exc == ham );
54+
} else {
55+
CHECK( exc + 1 <= ham );
56+
}
5357
CHECK( ham <= 2 * exc );
5458

5559
CHECK( max <= dis );

0 commit comments

Comments
 (0)