Skip to content

Commit a89ca18

Browse files
committed
Provide const dereference of filter / concat iterators only
Deferencing a `const` iterator which refers to a non-const value should enable mutation of that value. This is consistent with how iterators in the STL work. It means that we only need to provide a single const overload of these operators, not both the const and non-const variations. So the benefit of this commit is better consistency with the STL and fewer operator overloads to maintain.
1 parent 3da7082 commit a89ca18

File tree

2 files changed

+36
-28
lines changed

2 files changed

+36
-28
lines changed

src/util/range.h

Lines changed: 4 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -143,22 +143,12 @@ class filter_iteratort
143143
return tmp;
144144
}
145145

146-
reference operator*()
146+
reference operator*() const
147147
{
148148
return *underlying;
149149
}
150150

151-
pointer operator->()
152-
{
153-
return &(*underlying);
154-
}
155-
156-
const value_type &operator*() const
157-
{
158-
return *underlying;
159-
}
160-
161-
const value_type *operator->() const
151+
pointer operator->() const
162152
{
163153
return &(*underlying);
164154
}
@@ -245,28 +235,14 @@ struct concat_iteratort
245235
return tmp;
246236
}
247237

248-
reference operator*()
249-
{
250-
if(first_begin == first_end)
251-
return *second_begin;
252-
return *first_begin;
253-
}
254-
255-
pointer operator->()
256-
{
257-
if(first_begin == first_end)
258-
return &(*second_begin);
259-
return &(*first_begin);
260-
}
261-
262-
const value_type &operator*() const
238+
reference operator*() const
263239
{
264240
if(first_begin == first_end)
265241
return *second_begin;
266242
return *first_begin;
267243
}
268244

269-
const value_type *operator->() const
245+
pointer operator->() const
270246
{
271247
if(first_begin == first_end)
272248
return &(*second_begin);

unit/util/range.cpp

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,17 @@ SCENARIO("range tests", "[core][util][range]")
5757
++it;
5858
REQUIRE(it == filtered_range.end());
5959
}
60+
THEN(
61+
"A const instance of a `filter_iteratort` can mutate the input "
62+
"collection.")
63+
{
64+
const auto it =
65+
make_range(list)
66+
.filter([&](const std::string &s) { return s.length() == 3; })
67+
.begin();
68+
*it += "x";
69+
REQUIRE(*list.begin() == "abcx");
70+
}
6071
THEN("Filter, map and use range-for on the same list")
6172
{
6273
const auto range =
@@ -117,6 +128,27 @@ SCENARIO("range tests", "[core][util][range]")
117128
REQUIRE(output == expected);
118129
};
119130
}
131+
GIVEN("Two non-const vectors of ints.")
132+
{
133+
std::vector<int> input1{1, 2};
134+
std::vector<int> input2{3, 4};
135+
THEN(
136+
"Const instances of `concat_iteratort` should enable the input "
137+
"collections to be mutated.")
138+
{
139+
const auto concat_range = make_range(input1).concat(make_range(input2));
140+
int x = 5;
141+
for(auto it = concat_range.begin(); it != concat_range.end(); ++it, ++x)
142+
{
143+
const auto const_it = it;
144+
*const_it = x;
145+
}
146+
std::vector<int> expected_result1{5, 6};
147+
std::vector<int> expected_result2{7, 8};
148+
REQUIRE(input1 == expected_result1);
149+
REQUIRE(input2 == expected_result2);
150+
}
151+
}
120152
}
121153

122154
class move_onlyt

0 commit comments

Comments
 (0)