patternpythonMinor
Python reversed in C++11
Viewed 0 times
reversedpythonstackoverflow
Problem
I was playing with some of the wonderful new C++11 features and tried to reimplement a function
And to clarify since I'm been told that I was unclear, that
Here's my code:
```
template
class ReversedObject
{
private:
BidirectionalIterable& _iter;
ReversedObject(BidirectionalIterable&& iter):
_iter(iter)
{}
public:
using value_type = typename std::decay::type;
using difference_type = std::ptrdiff_t;
using reference = value_type&;
using pointer = value_type*;
using iterator = typename std::remove_reference::type;
using const_iterator = typename std::remove_reference::type;
using reverse_iterator = decltype(std::begin(_iter));
using const_reverse_iterator = decltype(std::begin(_iter));
using iterator_category = typename std::iterator_traits::iterator_category;
// Iterator functions
auto begin() -> iterator
{ return itertools::rbegin(_iter); }
auto begin() const -> const_iterator
{ return itertools::rbegin(_iter); }
auto cbegin() const -> const_iterator
{ return itertools::rbegin(_iter); }
auto end() -> iterator
{ return itertools::rend(_iter); }
auto end() const -> const_iterator
{ return itertools::rend(_iter); }
auto cend() const -> const_iterator
{ return itertools::rend(_iter); }
// Reverse iterator functions
auto rbegin() -> reverse_iterator
{ return std::begin(_iter); }
auto rbegin() const -> const_reverse_iterator
reversed that would behave just like the one in Python to allow reverse iteration on bidirectional iterables.And to clarify since I'm been told that I was unclear, that
reverse function would be used in range-based for loops. Also, I define an iterator as whatever satisfies the functions std::begin and std::end (ForwardIterator concept). I don't think I've got to explain what a bidirectional iterator is... :DHere's my code:
```
template
class ReversedObject
{
private:
BidirectionalIterable& _iter;
ReversedObject(BidirectionalIterable&& iter):
_iter(iter)
{}
public:
using value_type = typename std::decay::type;
using difference_type = std::ptrdiff_t;
using reference = value_type&;
using pointer = value_type*;
using iterator = typename std::remove_reference::type;
using const_iterator = typename std::remove_reference::type;
using reverse_iterator = decltype(std::begin(_iter));
using const_reverse_iterator = decltype(std::begin(_iter));
using iterator_category = typename std::iterator_traits::iterator_category;
// Iterator functions
auto begin() -> iterator
{ return itertools::rbegin(_iter); }
auto begin() const -> const_iterator
{ return itertools::rbegin(_iter); }
auto cbegin() const -> const_iterator
{ return itertools::rbegin(_iter); }
auto end() -> iterator
{ return itertools::rend(_iter); }
auto end() const -> const_iterator
{ return itertools::rend(_iter); }
auto cend() const -> const_iterator
{ return itertools::rend(_iter); }
// Reverse iterator functions
auto rbegin() -> reverse_iterator
{ return std::begin(_iter); }
auto rbegin() const -> const_reverse_iterator
Solution
Overall I like this helper. Most potential complaints are due to evaluating how well it would handle something it wasn't intended to support. Here are some of my thoughts from examining its implementation.
This code goes out of its way to define a lot of names for types that are not used directly by the consuming code (
There's no specialization for
As other commenters have said, calling it
It would be interesting to try to reveal more of the underlying container per Dave's comment about a subset of a vector's functionality. If this is only ever used directly in a range-based for loop, that's not too important. But if someone starts saving reversed containers (it could be interesting to have a vector that effectively has a
I do like the fact that trying to reverse something that doesn't have reverse iterators will fail at compile time, even if its iterators are not invoked. The following gave me clear compilation errors:
I don't like the
Thanks for the interesting code to review. It makes me want to try to write a
This code goes out of its way to define a lot of names for types that are not used directly by the consuming code (
value_type, difference_type, etc.), nor by my understanding of the mechanisms of range-based for. Yet it doesn't offer an explicit overload for std::begin and friends.There's no specialization for
reversed(reversed(container)). While this is probably fine because it's unlikely to come up, and likely to be inlined down to the original container, I almost expected to see something like this explicitly cover this case:template
inline auto reversed(ReversedObject&& iter)
-> BidirectionalIterable
{
return { std::forward(iter._iter) };
}As other commenters have said, calling it
iter in a c++ world is a bit confusing. Then again calling it iter in python would be stomping on the builtin. So I would recommend renaming it iterable or container.It would be interesting to try to reveal more of the underlying container per Dave's comment about a subset of a vector's functionality. If this is only ever used directly in a range-based for loop, that's not too important. But if someone starts saving reversed containers (it could be interesting to have a vector that effectively has a
push_front instead of a push_back), it would become frustrating to lack other methods. Could an approach that derives from the reversed container do the trick? Hmm, probably not; this would typically result in copying the full data structure, operator[] would have to be adjusted, and data() would fall apart. But that thought about copying brings up another point. The name ReversedObject is misleading; it's more like a ReversedView.I do like the fact that trying to reverse something that doesn't have reverse iterators will fail at compile time, even if its iterators are not invoked. The following gave me clear compilation errors:
struct NonReversible {};
auto a = NonReversible();
auto b = reversed(a); // clear errors pointing hereI don't like the
itertools items as much. If we could just jump ahead to C++14, that'd be fine. Outside that, my first inclination is that I'd rather see this specialization done as a partial specialization of the ReversedObject class. But it's more reusable the way you did it, and the way C++ is going, so it's hard to justify my reaction.Thanks for the interesting code to review. It makes me want to try to write a
zip for range-based for loops. (Even though it's already been done too.)Code Snippets
template<typename BidirectionalIterable>
inline auto reversed(ReversedObject<BidirectionalIterable>&& iter)
-> BidirectionalIterable
{
return { std::forward<BidirectionalIterable>(iter._iter) };
}struct NonReversible {};
auto a = NonReversible();
auto b = reversed(a); // clear errors pointing hereContext
StackExchange Code Review Q#23734, answer score: 3
Revisions (0)
No revisions yet.