Custom C++ random access iterator for pointers to arrays
Clash Royale CLAN TAG#URR8PPP
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;
up vote
10
down vote
favorite
I have implemented a custom iterator to iterate over an array that is held by a pointer. The iterator accepts a unit which specifies by which amount the underlying pointer is increased (or decreased) at each increment (or decrement).
I.e. when
auto data = new int[100];
ArrayIterator<int, false> it(data, 2); // 2nd argument is unit
it + 1
refers to the same element as data + 2
.
PointerIterator.hpp
#pragma once
#include <iterator>
/**
* brief A class for iterating over pointers. This class allows to specify a unit, by which the underlying pointers are
* incremented (or decremented) when increasing (or decreasing) it.
* I.e. this allows to easily iterate over columns of a matrix that is represented by a single pointer in row major order.
* tparam DataT The data type the pointer is pointing to.
* tparam Reverse True, if the iterator is used as a reverse iterator (incrementing the iterator is decrementing the pointer, and vice versa).
*/
template <typename DataT, bool Reverse = false>
class ArrayIterator
public:
using difference_type = long long;
using value_type = DataT;
using pointer = DataT*;
using const_pointer = const DataT*;
using reference = DataT&;
using const_reference = const DataT&;
using iterator_category = std::random_access_iterator_tag;
using const_iterator = ArrayIterator<const std::remove_const_t<DataT>, Reverse>;
/**
* brief Creates an invalid iterator.
*/
ArrayIterator() = default;
/**
* brief Creates a pointer iterator for the specified pointer.
* tparam DataT The underlying data type of the pointer.
* param data The pointer.
* param unit The amount by which the pointer is increased (or decreased) at each increment (or decrement).
*/
explicit ArrayIterator(DataT* data, size_t unit = 1);
operator const_iterator() const; // allow implicit conversion from iterator to const iterator
ArrayIterator& operator++();
ArrayIterator operator++(int);
ArrayIterator& operator--();
ArrayIterator operator--(int);
ArrayIterator& operator+=(int n);
ArrayIterator& operator-=(int n);
ArrayIterator operator+(int n) const;
ArrayIterator operator-(int n) const;
difference_type operator-(ArrayIterator other) const;
reference operator(int n);
const_reference operator(int n) const;
bool operator<(ArrayIterator other) const;
bool operator<=(ArrayIterator other) const;
bool operator>(ArrayIterator other) const;
bool operator>=(ArrayIterator other) const;
bool operator==(ArrayIterator other) const;
bool operator!=(ArrayIterator other) const;
pointer operator->();
const_pointer operator->() const;
reference operator*();
const_reference operator*() const;
void swap(ArrayIterator& other) noexcept;
size_t getUnit() const;
private:
pointer _ptr = nullptr;
size_t _unit = 0;
;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse>::ArrayIterator(DataT* data, size_t unit)
: _ptr(data), _unit(unit)
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse>::operator const_iterator() const
using type = const std::remove_const_t<DataT>;
return ArrayIterator<type, Reverse>(reinterpret_cast<type*>(_ptr), _unit);
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse>& ArrayIterator<DataT, Reverse>::operator++()
if constexpr (Reverse)
_ptr -= _unit;
else
_ptr += _unit;
return *this;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse> ArrayIterator<DataT, Reverse>::operator++(int)
auto it = *this;
++*this;
return it;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse>& ArrayIterator<DataT, Reverse>::operator--()
if constexpr (Reverse)
_ptr += _unit;
else
_ptr -= _unit;
return *this;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse> ArrayIterator<DataT, Reverse>::operator--(int)
auto it = *this;
--*this;
return it;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse>& ArrayIterator<DataT, Reverse>::operator+=(int n)
if constexpr (Reverse)
_ptr -= n * _unit;
else
_ptr += n * _unit;
return *this;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse>& ArrayIterator<DataT, Reverse>::operator-=(int n)
if constexpr (Reverse)
_ptr += n * _unit;
else
_ptr -= n * _unit;
return *this;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse> ArrayIterator<DataT, Reverse>::operator+(int n) const
auto it = *this;
it += n;
return it;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse> ArrayIterator<DataT, Reverse>::operator-(int n) const
auto it = *this;
it -= n;
return it;
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::difference_type ArrayIterator<DataT, Reverse>::operator-(ArrayIterator other) const
if (_unit != other._unit)
throw std::logic_error("Cannot calculate differnce for iterators with different unit.");
if constexpr (Reverse)
return (_ptr - other._ptr) / _unit;
else
return (other._ptr - _ptr) / _unit;
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::reference ArrayIterator<DataT, Reverse>::operator(int n)
return *(*this + n);
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::const_reference ArrayIterator<DataT, Reverse>::operator(int n) const
return *(*this + n);
template <typename DataT, bool Reverse>
bool ArrayIterator<DataT, Reverse>::operator<(ArrayIterator other) const
if constexpr (Reverse)
return other._ptr < _ptr;
else
return _ptr < other._ptr;
template <typename DataT, bool Reverse>
bool ArrayIterator<DataT, Reverse>::operator<=(ArrayIterator other) const
if constexpr (Reverse)
return other._ptr <= _ptr;
else
return _ptr <= other._ptr;
template <typename DataT, bool Reverse>
bool ArrayIterator<DataT, Reverse>::operator>(ArrayIterator other) const
if constexpr (Reverse)
return other._ptr > _ptr;
else
return _ptr > other._ptr;
template <typename DataT, bool Reverse>
bool ArrayIterator<DataT, Reverse>::operator>=(ArrayIterator other) const
if constexpr (Reverse)
return other._ptr >= _ptr;
else
return _ptr >= other._ptr;
template <typename DataT, bool Reverse>
bool ArrayIterator<DataT, Reverse>::operator==(ArrayIterator other) const
return _ptr == other._ptr;
template <typename DataT, bool Reverse>
bool ArrayIterator<DataT, Reverse>::operator!=(ArrayIterator other) const
return !(*this == other);
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::pointer ArrayIterator<DataT, Reverse>::operator->()
return _ptr;
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::const_pointer ArrayIterator<DataT, Reverse>::operator->() const
return _ptr;
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::reference ArrayIterator<DataT, Reverse>::operator*()
return *_ptr;
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::const_reference ArrayIterator<DataT, Reverse>::operator*() const
return *_ptr;
template <typename DataT, bool Reverse>
void ArrayIterator<DataT, Reverse>::swap(ArrayIterator& other) noexcept
auto mPtr = _ptr;
auto mUnit = _unit;
_ptr = other._ptr;
_unit = other._unit;
other._ptr = mPtr;
other._unit = mUnit;
template <typename DataT, bool Reverse>
size_t ArrayIterator<DataT, Reverse>::getUnit() const
return _unit;
/**
* brief Creates an iterator for the specified pointer.
* tparam DataT The underlying data type of the pointer.
* param ptr The pointer.
* param unit The amount by which the pointer is increased (or decreased) at each increment (or decrement).
*/
template <typename DataT>
ArrayIterator<DataT, false> makeArrayIterator(DataT* ptr, size_t unit = 1)
return ArrayIterator<DataT, false>(ptr, unit);
/**
* brief Creates a reversed iterator for the specified pointer. That is, if the iterator is increased by
* an arbitrary amount x, the underlying pointer is decreased by x (and vice versa).
* tparam DataT The underlying data type of the pointer.
* param ptr The pointer.
* param unit The amount by which the pointer is increased (or decreased) at each increment (or decrement).
*/
template <typename DataT>
ArrayIterator<DataT, true> makeArrayIterator(DataT* ptr, size_t unit = 1)
return ArrayIterator<DataT, true>(ptr, unit);
/**
* brief Creates a const iterator for the specified pointer.
* tparam DataT The underlying data type of the pointer.
* param ptr The pointer.
* param unit The amount by which the pointer is increased (or decreased) at each increment (or decrement).
*/
template <typename DataT>
ArrayIterator<DataT, false> makeConstArrayIterator(DataT* ptr, size_t unit = 1)
return ArrayIterator<const std::remove_const_t<DataT>, false>(ptr, unit);
/**
* brief Creates a const reversed iterator for the specified pointer. That is, if the iterator is increased by
* an arbitrary amount x, the underlying pointer is decreased by x (and vice versa).
* tparam DataT The underlying data type of the pointer.
* param ptr The pointer.
* param unit The amount by which the pointer is increased (or decreased) at each increment (or decrement).
*/
template <typename DataT>
ArrayIterator<const DataT, true> makeConstReverseArrayIterator(DataT* ptr, size_t unit = 1)
return ArrayIterator<const std::remove_const_t<DataT>, true>(ptr, unit);
Are there any mistakes or improvements?
c++ iterator c++17
 |Â
show 1 more comment
up vote
10
down vote
favorite
I have implemented a custom iterator to iterate over an array that is held by a pointer. The iterator accepts a unit which specifies by which amount the underlying pointer is increased (or decreased) at each increment (or decrement).
I.e. when
auto data = new int[100];
ArrayIterator<int, false> it(data, 2); // 2nd argument is unit
it + 1
refers to the same element as data + 2
.
PointerIterator.hpp
#pragma once
#include <iterator>
/**
* brief A class for iterating over pointers. This class allows to specify a unit, by which the underlying pointers are
* incremented (or decremented) when increasing (or decreasing) it.
* I.e. this allows to easily iterate over columns of a matrix that is represented by a single pointer in row major order.
* tparam DataT The data type the pointer is pointing to.
* tparam Reverse True, if the iterator is used as a reverse iterator (incrementing the iterator is decrementing the pointer, and vice versa).
*/
template <typename DataT, bool Reverse = false>
class ArrayIterator
public:
using difference_type = long long;
using value_type = DataT;
using pointer = DataT*;
using const_pointer = const DataT*;
using reference = DataT&;
using const_reference = const DataT&;
using iterator_category = std::random_access_iterator_tag;
using const_iterator = ArrayIterator<const std::remove_const_t<DataT>, Reverse>;
/**
* brief Creates an invalid iterator.
*/
ArrayIterator() = default;
/**
* brief Creates a pointer iterator for the specified pointer.
* tparam DataT The underlying data type of the pointer.
* param data The pointer.
* param unit The amount by which the pointer is increased (or decreased) at each increment (or decrement).
*/
explicit ArrayIterator(DataT* data, size_t unit = 1);
operator const_iterator() const; // allow implicit conversion from iterator to const iterator
ArrayIterator& operator++();
ArrayIterator operator++(int);
ArrayIterator& operator--();
ArrayIterator operator--(int);
ArrayIterator& operator+=(int n);
ArrayIterator& operator-=(int n);
ArrayIterator operator+(int n) const;
ArrayIterator operator-(int n) const;
difference_type operator-(ArrayIterator other) const;
reference operator(int n);
const_reference operator(int n) const;
bool operator<(ArrayIterator other) const;
bool operator<=(ArrayIterator other) const;
bool operator>(ArrayIterator other) const;
bool operator>=(ArrayIterator other) const;
bool operator==(ArrayIterator other) const;
bool operator!=(ArrayIterator other) const;
pointer operator->();
const_pointer operator->() const;
reference operator*();
const_reference operator*() const;
void swap(ArrayIterator& other) noexcept;
size_t getUnit() const;
private:
pointer _ptr = nullptr;
size_t _unit = 0;
;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse>::ArrayIterator(DataT* data, size_t unit)
: _ptr(data), _unit(unit)
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse>::operator const_iterator() const
using type = const std::remove_const_t<DataT>;
return ArrayIterator<type, Reverse>(reinterpret_cast<type*>(_ptr), _unit);
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse>& ArrayIterator<DataT, Reverse>::operator++()
if constexpr (Reverse)
_ptr -= _unit;
else
_ptr += _unit;
return *this;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse> ArrayIterator<DataT, Reverse>::operator++(int)
auto it = *this;
++*this;
return it;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse>& ArrayIterator<DataT, Reverse>::operator--()
if constexpr (Reverse)
_ptr += _unit;
else
_ptr -= _unit;
return *this;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse> ArrayIterator<DataT, Reverse>::operator--(int)
auto it = *this;
--*this;
return it;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse>& ArrayIterator<DataT, Reverse>::operator+=(int n)
if constexpr (Reverse)
_ptr -= n * _unit;
else
_ptr += n * _unit;
return *this;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse>& ArrayIterator<DataT, Reverse>::operator-=(int n)
if constexpr (Reverse)
_ptr += n * _unit;
else
_ptr -= n * _unit;
return *this;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse> ArrayIterator<DataT, Reverse>::operator+(int n) const
auto it = *this;
it += n;
return it;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse> ArrayIterator<DataT, Reverse>::operator-(int n) const
auto it = *this;
it -= n;
return it;
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::difference_type ArrayIterator<DataT, Reverse>::operator-(ArrayIterator other) const
if (_unit != other._unit)
throw std::logic_error("Cannot calculate differnce for iterators with different unit.");
if constexpr (Reverse)
return (_ptr - other._ptr) / _unit;
else
return (other._ptr - _ptr) / _unit;
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::reference ArrayIterator<DataT, Reverse>::operator(int n)
return *(*this + n);
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::const_reference ArrayIterator<DataT, Reverse>::operator(int n) const
return *(*this + n);
template <typename DataT, bool Reverse>
bool ArrayIterator<DataT, Reverse>::operator<(ArrayIterator other) const
if constexpr (Reverse)
return other._ptr < _ptr;
else
return _ptr < other._ptr;
template <typename DataT, bool Reverse>
bool ArrayIterator<DataT, Reverse>::operator<=(ArrayIterator other) const
if constexpr (Reverse)
return other._ptr <= _ptr;
else
return _ptr <= other._ptr;
template <typename DataT, bool Reverse>
bool ArrayIterator<DataT, Reverse>::operator>(ArrayIterator other) const
if constexpr (Reverse)
return other._ptr > _ptr;
else
return _ptr > other._ptr;
template <typename DataT, bool Reverse>
bool ArrayIterator<DataT, Reverse>::operator>=(ArrayIterator other) const
if constexpr (Reverse)
return other._ptr >= _ptr;
else
return _ptr >= other._ptr;
template <typename DataT, bool Reverse>
bool ArrayIterator<DataT, Reverse>::operator==(ArrayIterator other) const
return _ptr == other._ptr;
template <typename DataT, bool Reverse>
bool ArrayIterator<DataT, Reverse>::operator!=(ArrayIterator other) const
return !(*this == other);
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::pointer ArrayIterator<DataT, Reverse>::operator->()
return _ptr;
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::const_pointer ArrayIterator<DataT, Reverse>::operator->() const
return _ptr;
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::reference ArrayIterator<DataT, Reverse>::operator*()
return *_ptr;
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::const_reference ArrayIterator<DataT, Reverse>::operator*() const
return *_ptr;
template <typename DataT, bool Reverse>
void ArrayIterator<DataT, Reverse>::swap(ArrayIterator& other) noexcept
auto mPtr = _ptr;
auto mUnit = _unit;
_ptr = other._ptr;
_unit = other._unit;
other._ptr = mPtr;
other._unit = mUnit;
template <typename DataT, bool Reverse>
size_t ArrayIterator<DataT, Reverse>::getUnit() const
return _unit;
/**
* brief Creates an iterator for the specified pointer.
* tparam DataT The underlying data type of the pointer.
* param ptr The pointer.
* param unit The amount by which the pointer is increased (or decreased) at each increment (or decrement).
*/
template <typename DataT>
ArrayIterator<DataT, false> makeArrayIterator(DataT* ptr, size_t unit = 1)
return ArrayIterator<DataT, false>(ptr, unit);
/**
* brief Creates a reversed iterator for the specified pointer. That is, if the iterator is increased by
* an arbitrary amount x, the underlying pointer is decreased by x (and vice versa).
* tparam DataT The underlying data type of the pointer.
* param ptr The pointer.
* param unit The amount by which the pointer is increased (or decreased) at each increment (or decrement).
*/
template <typename DataT>
ArrayIterator<DataT, true> makeArrayIterator(DataT* ptr, size_t unit = 1)
return ArrayIterator<DataT, true>(ptr, unit);
/**
* brief Creates a const iterator for the specified pointer.
* tparam DataT The underlying data type of the pointer.
* param ptr The pointer.
* param unit The amount by which the pointer is increased (or decreased) at each increment (or decrement).
*/
template <typename DataT>
ArrayIterator<DataT, false> makeConstArrayIterator(DataT* ptr, size_t unit = 1)
return ArrayIterator<const std::remove_const_t<DataT>, false>(ptr, unit);
/**
* brief Creates a const reversed iterator for the specified pointer. That is, if the iterator is increased by
* an arbitrary amount x, the underlying pointer is decreased by x (and vice versa).
* tparam DataT The underlying data type of the pointer.
* param ptr The pointer.
* param unit The amount by which the pointer is increased (or decreased) at each increment (or decrement).
*/
template <typename DataT>
ArrayIterator<const DataT, true> makeConstReverseArrayIterator(DataT* ptr, size_t unit = 1)
return ArrayIterator<const std::remove_const_t<DataT>, true>(ptr, unit);
Are there any mistakes or improvements?
c++ iterator c++17
1
makeConstArrayIterator
with direction forward is missing aconst
on the 1st template argument for the return type.
â Darhuuk
Apr 16 at 13:31
@Darhuuk Thanks, fixed it.
â Timo
Apr 16 at 13:37
1
This seems an oddly specific template. A pointer-into-array is aRandomAccessIterator
. I would generalise this to adapt anyRandomAccessIterator
â Caleth
Apr 16 at 17:44
@Caleth That is indeed a good point.
â Timo
Apr 16 at 18:56
Why is units a constructor parameter and not a template parameter. You mention that pointers with different units are not comparable. So why not make them distinct types by putting the size into the template. It also reduces the size of your iterator object.
â Martin York
Apr 16 at 22:09
 |Â
show 1 more comment
up vote
10
down vote
favorite
up vote
10
down vote
favorite
I have implemented a custom iterator to iterate over an array that is held by a pointer. The iterator accepts a unit which specifies by which amount the underlying pointer is increased (or decreased) at each increment (or decrement).
I.e. when
auto data = new int[100];
ArrayIterator<int, false> it(data, 2); // 2nd argument is unit
it + 1
refers to the same element as data + 2
.
PointerIterator.hpp
#pragma once
#include <iterator>
/**
* brief A class for iterating over pointers. This class allows to specify a unit, by which the underlying pointers are
* incremented (or decremented) when increasing (or decreasing) it.
* I.e. this allows to easily iterate over columns of a matrix that is represented by a single pointer in row major order.
* tparam DataT The data type the pointer is pointing to.
* tparam Reverse True, if the iterator is used as a reverse iterator (incrementing the iterator is decrementing the pointer, and vice versa).
*/
template <typename DataT, bool Reverse = false>
class ArrayIterator
public:
using difference_type = long long;
using value_type = DataT;
using pointer = DataT*;
using const_pointer = const DataT*;
using reference = DataT&;
using const_reference = const DataT&;
using iterator_category = std::random_access_iterator_tag;
using const_iterator = ArrayIterator<const std::remove_const_t<DataT>, Reverse>;
/**
* brief Creates an invalid iterator.
*/
ArrayIterator() = default;
/**
* brief Creates a pointer iterator for the specified pointer.
* tparam DataT The underlying data type of the pointer.
* param data The pointer.
* param unit The amount by which the pointer is increased (or decreased) at each increment (or decrement).
*/
explicit ArrayIterator(DataT* data, size_t unit = 1);
operator const_iterator() const; // allow implicit conversion from iterator to const iterator
ArrayIterator& operator++();
ArrayIterator operator++(int);
ArrayIterator& operator--();
ArrayIterator operator--(int);
ArrayIterator& operator+=(int n);
ArrayIterator& operator-=(int n);
ArrayIterator operator+(int n) const;
ArrayIterator operator-(int n) const;
difference_type operator-(ArrayIterator other) const;
reference operator(int n);
const_reference operator(int n) const;
bool operator<(ArrayIterator other) const;
bool operator<=(ArrayIterator other) const;
bool operator>(ArrayIterator other) const;
bool operator>=(ArrayIterator other) const;
bool operator==(ArrayIterator other) const;
bool operator!=(ArrayIterator other) const;
pointer operator->();
const_pointer operator->() const;
reference operator*();
const_reference operator*() const;
void swap(ArrayIterator& other) noexcept;
size_t getUnit() const;
private:
pointer _ptr = nullptr;
size_t _unit = 0;
;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse>::ArrayIterator(DataT* data, size_t unit)
: _ptr(data), _unit(unit)
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse>::operator const_iterator() const
using type = const std::remove_const_t<DataT>;
return ArrayIterator<type, Reverse>(reinterpret_cast<type*>(_ptr), _unit);
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse>& ArrayIterator<DataT, Reverse>::operator++()
if constexpr (Reverse)
_ptr -= _unit;
else
_ptr += _unit;
return *this;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse> ArrayIterator<DataT, Reverse>::operator++(int)
auto it = *this;
++*this;
return it;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse>& ArrayIterator<DataT, Reverse>::operator--()
if constexpr (Reverse)
_ptr += _unit;
else
_ptr -= _unit;
return *this;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse> ArrayIterator<DataT, Reverse>::operator--(int)
auto it = *this;
--*this;
return it;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse>& ArrayIterator<DataT, Reverse>::operator+=(int n)
if constexpr (Reverse)
_ptr -= n * _unit;
else
_ptr += n * _unit;
return *this;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse>& ArrayIterator<DataT, Reverse>::operator-=(int n)
if constexpr (Reverse)
_ptr += n * _unit;
else
_ptr -= n * _unit;
return *this;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse> ArrayIterator<DataT, Reverse>::operator+(int n) const
auto it = *this;
it += n;
return it;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse> ArrayIterator<DataT, Reverse>::operator-(int n) const
auto it = *this;
it -= n;
return it;
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::difference_type ArrayIterator<DataT, Reverse>::operator-(ArrayIterator other) const
if (_unit != other._unit)
throw std::logic_error("Cannot calculate differnce for iterators with different unit.");
if constexpr (Reverse)
return (_ptr - other._ptr) / _unit;
else
return (other._ptr - _ptr) / _unit;
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::reference ArrayIterator<DataT, Reverse>::operator(int n)
return *(*this + n);
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::const_reference ArrayIterator<DataT, Reverse>::operator(int n) const
return *(*this + n);
template <typename DataT, bool Reverse>
bool ArrayIterator<DataT, Reverse>::operator<(ArrayIterator other) const
if constexpr (Reverse)
return other._ptr < _ptr;
else
return _ptr < other._ptr;
template <typename DataT, bool Reverse>
bool ArrayIterator<DataT, Reverse>::operator<=(ArrayIterator other) const
if constexpr (Reverse)
return other._ptr <= _ptr;
else
return _ptr <= other._ptr;
template <typename DataT, bool Reverse>
bool ArrayIterator<DataT, Reverse>::operator>(ArrayIterator other) const
if constexpr (Reverse)
return other._ptr > _ptr;
else
return _ptr > other._ptr;
template <typename DataT, bool Reverse>
bool ArrayIterator<DataT, Reverse>::operator>=(ArrayIterator other) const
if constexpr (Reverse)
return other._ptr >= _ptr;
else
return _ptr >= other._ptr;
template <typename DataT, bool Reverse>
bool ArrayIterator<DataT, Reverse>::operator==(ArrayIterator other) const
return _ptr == other._ptr;
template <typename DataT, bool Reverse>
bool ArrayIterator<DataT, Reverse>::operator!=(ArrayIterator other) const
return !(*this == other);
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::pointer ArrayIterator<DataT, Reverse>::operator->()
return _ptr;
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::const_pointer ArrayIterator<DataT, Reverse>::operator->() const
return _ptr;
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::reference ArrayIterator<DataT, Reverse>::operator*()
return *_ptr;
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::const_reference ArrayIterator<DataT, Reverse>::operator*() const
return *_ptr;
template <typename DataT, bool Reverse>
void ArrayIterator<DataT, Reverse>::swap(ArrayIterator& other) noexcept
auto mPtr = _ptr;
auto mUnit = _unit;
_ptr = other._ptr;
_unit = other._unit;
other._ptr = mPtr;
other._unit = mUnit;
template <typename DataT, bool Reverse>
size_t ArrayIterator<DataT, Reverse>::getUnit() const
return _unit;
/**
* brief Creates an iterator for the specified pointer.
* tparam DataT The underlying data type of the pointer.
* param ptr The pointer.
* param unit The amount by which the pointer is increased (or decreased) at each increment (or decrement).
*/
template <typename DataT>
ArrayIterator<DataT, false> makeArrayIterator(DataT* ptr, size_t unit = 1)
return ArrayIterator<DataT, false>(ptr, unit);
/**
* brief Creates a reversed iterator for the specified pointer. That is, if the iterator is increased by
* an arbitrary amount x, the underlying pointer is decreased by x (and vice versa).
* tparam DataT The underlying data type of the pointer.
* param ptr The pointer.
* param unit The amount by which the pointer is increased (or decreased) at each increment (or decrement).
*/
template <typename DataT>
ArrayIterator<DataT, true> makeArrayIterator(DataT* ptr, size_t unit = 1)
return ArrayIterator<DataT, true>(ptr, unit);
/**
* brief Creates a const iterator for the specified pointer.
* tparam DataT The underlying data type of the pointer.
* param ptr The pointer.
* param unit The amount by which the pointer is increased (or decreased) at each increment (or decrement).
*/
template <typename DataT>
ArrayIterator<DataT, false> makeConstArrayIterator(DataT* ptr, size_t unit = 1)
return ArrayIterator<const std::remove_const_t<DataT>, false>(ptr, unit);
/**
* brief Creates a const reversed iterator for the specified pointer. That is, if the iterator is increased by
* an arbitrary amount x, the underlying pointer is decreased by x (and vice versa).
* tparam DataT The underlying data type of the pointer.
* param ptr The pointer.
* param unit The amount by which the pointer is increased (or decreased) at each increment (or decrement).
*/
template <typename DataT>
ArrayIterator<const DataT, true> makeConstReverseArrayIterator(DataT* ptr, size_t unit = 1)
return ArrayIterator<const std::remove_const_t<DataT>, true>(ptr, unit);
Are there any mistakes or improvements?
c++ iterator c++17
I have implemented a custom iterator to iterate over an array that is held by a pointer. The iterator accepts a unit which specifies by which amount the underlying pointer is increased (or decreased) at each increment (or decrement).
I.e. when
auto data = new int[100];
ArrayIterator<int, false> it(data, 2); // 2nd argument is unit
it + 1
refers to the same element as data + 2
.
PointerIterator.hpp
#pragma once
#include <iterator>
/**
* brief A class for iterating over pointers. This class allows to specify a unit, by which the underlying pointers are
* incremented (or decremented) when increasing (or decreasing) it.
* I.e. this allows to easily iterate over columns of a matrix that is represented by a single pointer in row major order.
* tparam DataT The data type the pointer is pointing to.
* tparam Reverse True, if the iterator is used as a reverse iterator (incrementing the iterator is decrementing the pointer, and vice versa).
*/
template <typename DataT, bool Reverse = false>
class ArrayIterator
public:
using difference_type = long long;
using value_type = DataT;
using pointer = DataT*;
using const_pointer = const DataT*;
using reference = DataT&;
using const_reference = const DataT&;
using iterator_category = std::random_access_iterator_tag;
using const_iterator = ArrayIterator<const std::remove_const_t<DataT>, Reverse>;
/**
* brief Creates an invalid iterator.
*/
ArrayIterator() = default;
/**
* brief Creates a pointer iterator for the specified pointer.
* tparam DataT The underlying data type of the pointer.
* param data The pointer.
* param unit The amount by which the pointer is increased (or decreased) at each increment (or decrement).
*/
explicit ArrayIterator(DataT* data, size_t unit = 1);
operator const_iterator() const; // allow implicit conversion from iterator to const iterator
ArrayIterator& operator++();
ArrayIterator operator++(int);
ArrayIterator& operator--();
ArrayIterator operator--(int);
ArrayIterator& operator+=(int n);
ArrayIterator& operator-=(int n);
ArrayIterator operator+(int n) const;
ArrayIterator operator-(int n) const;
difference_type operator-(ArrayIterator other) const;
reference operator(int n);
const_reference operator(int n) const;
bool operator<(ArrayIterator other) const;
bool operator<=(ArrayIterator other) const;
bool operator>(ArrayIterator other) const;
bool operator>=(ArrayIterator other) const;
bool operator==(ArrayIterator other) const;
bool operator!=(ArrayIterator other) const;
pointer operator->();
const_pointer operator->() const;
reference operator*();
const_reference operator*() const;
void swap(ArrayIterator& other) noexcept;
size_t getUnit() const;
private:
pointer _ptr = nullptr;
size_t _unit = 0;
;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse>::ArrayIterator(DataT* data, size_t unit)
: _ptr(data), _unit(unit)
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse>::operator const_iterator() const
using type = const std::remove_const_t<DataT>;
return ArrayIterator<type, Reverse>(reinterpret_cast<type*>(_ptr), _unit);
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse>& ArrayIterator<DataT, Reverse>::operator++()
if constexpr (Reverse)
_ptr -= _unit;
else
_ptr += _unit;
return *this;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse> ArrayIterator<DataT, Reverse>::operator++(int)
auto it = *this;
++*this;
return it;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse>& ArrayIterator<DataT, Reverse>::operator--()
if constexpr (Reverse)
_ptr += _unit;
else
_ptr -= _unit;
return *this;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse> ArrayIterator<DataT, Reverse>::operator--(int)
auto it = *this;
--*this;
return it;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse>& ArrayIterator<DataT, Reverse>::operator+=(int n)
if constexpr (Reverse)
_ptr -= n * _unit;
else
_ptr += n * _unit;
return *this;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse>& ArrayIterator<DataT, Reverse>::operator-=(int n)
if constexpr (Reverse)
_ptr += n * _unit;
else
_ptr -= n * _unit;
return *this;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse> ArrayIterator<DataT, Reverse>::operator+(int n) const
auto it = *this;
it += n;
return it;
template <typename DataT, bool Reverse>
ArrayIterator<DataT, Reverse> ArrayIterator<DataT, Reverse>::operator-(int n) const
auto it = *this;
it -= n;
return it;
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::difference_type ArrayIterator<DataT, Reverse>::operator-(ArrayIterator other) const
if (_unit != other._unit)
throw std::logic_error("Cannot calculate differnce for iterators with different unit.");
if constexpr (Reverse)
return (_ptr - other._ptr) / _unit;
else
return (other._ptr - _ptr) / _unit;
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::reference ArrayIterator<DataT, Reverse>::operator(int n)
return *(*this + n);
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::const_reference ArrayIterator<DataT, Reverse>::operator(int n) const
return *(*this + n);
template <typename DataT, bool Reverse>
bool ArrayIterator<DataT, Reverse>::operator<(ArrayIterator other) const
if constexpr (Reverse)
return other._ptr < _ptr;
else
return _ptr < other._ptr;
template <typename DataT, bool Reverse>
bool ArrayIterator<DataT, Reverse>::operator<=(ArrayIterator other) const
if constexpr (Reverse)
return other._ptr <= _ptr;
else
return _ptr <= other._ptr;
template <typename DataT, bool Reverse>
bool ArrayIterator<DataT, Reverse>::operator>(ArrayIterator other) const
if constexpr (Reverse)
return other._ptr > _ptr;
else
return _ptr > other._ptr;
template <typename DataT, bool Reverse>
bool ArrayIterator<DataT, Reverse>::operator>=(ArrayIterator other) const
if constexpr (Reverse)
return other._ptr >= _ptr;
else
return _ptr >= other._ptr;
template <typename DataT, bool Reverse>
bool ArrayIterator<DataT, Reverse>::operator==(ArrayIterator other) const
return _ptr == other._ptr;
template <typename DataT, bool Reverse>
bool ArrayIterator<DataT, Reverse>::operator!=(ArrayIterator other) const
return !(*this == other);
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::pointer ArrayIterator<DataT, Reverse>::operator->()
return _ptr;
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::const_pointer ArrayIterator<DataT, Reverse>::operator->() const
return _ptr;
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::reference ArrayIterator<DataT, Reverse>::operator*()
return *_ptr;
template <typename DataT, bool Reverse>
typename ArrayIterator<DataT, Reverse>::const_reference ArrayIterator<DataT, Reverse>::operator*() const
return *_ptr;
template <typename DataT, bool Reverse>
void ArrayIterator<DataT, Reverse>::swap(ArrayIterator& other) noexcept
auto mPtr = _ptr;
auto mUnit = _unit;
_ptr = other._ptr;
_unit = other._unit;
other._ptr = mPtr;
other._unit = mUnit;
template <typename DataT, bool Reverse>
size_t ArrayIterator<DataT, Reverse>::getUnit() const
return _unit;
/**
* brief Creates an iterator for the specified pointer.
* tparam DataT The underlying data type of the pointer.
* param ptr The pointer.
* param unit The amount by which the pointer is increased (or decreased) at each increment (or decrement).
*/
template <typename DataT>
ArrayIterator<DataT, false> makeArrayIterator(DataT* ptr, size_t unit = 1)
return ArrayIterator<DataT, false>(ptr, unit);
/**
* brief Creates a reversed iterator for the specified pointer. That is, if the iterator is increased by
* an arbitrary amount x, the underlying pointer is decreased by x (and vice versa).
* tparam DataT The underlying data type of the pointer.
* param ptr The pointer.
* param unit The amount by which the pointer is increased (or decreased) at each increment (or decrement).
*/
template <typename DataT>
ArrayIterator<DataT, true> makeArrayIterator(DataT* ptr, size_t unit = 1)
return ArrayIterator<DataT, true>(ptr, unit);
/**
* brief Creates a const iterator for the specified pointer.
* tparam DataT The underlying data type of the pointer.
* param ptr The pointer.
* param unit The amount by which the pointer is increased (or decreased) at each increment (or decrement).
*/
template <typename DataT>
ArrayIterator<DataT, false> makeConstArrayIterator(DataT* ptr, size_t unit = 1)
return ArrayIterator<const std::remove_const_t<DataT>, false>(ptr, unit);
/**
* brief Creates a const reversed iterator for the specified pointer. That is, if the iterator is increased by
* an arbitrary amount x, the underlying pointer is decreased by x (and vice versa).
* tparam DataT The underlying data type of the pointer.
* param ptr The pointer.
* param unit The amount by which the pointer is increased (or decreased) at each increment (or decrement).
*/
template <typename DataT>
ArrayIterator<const DataT, true> makeConstReverseArrayIterator(DataT* ptr, size_t unit = 1)
return ArrayIterator<const std::remove_const_t<DataT>, true>(ptr, unit);
Are there any mistakes or improvements?
c++ iterator c++17
edited Apr 16 at 15:17
asked Apr 16 at 12:05
Timo
1818
1818
1
makeConstArrayIterator
with direction forward is missing aconst
on the 1st template argument for the return type.
â Darhuuk
Apr 16 at 13:31
@Darhuuk Thanks, fixed it.
â Timo
Apr 16 at 13:37
1
This seems an oddly specific template. A pointer-into-array is aRandomAccessIterator
. I would generalise this to adapt anyRandomAccessIterator
â Caleth
Apr 16 at 17:44
@Caleth That is indeed a good point.
â Timo
Apr 16 at 18:56
Why is units a constructor parameter and not a template parameter. You mention that pointers with different units are not comparable. So why not make them distinct types by putting the size into the template. It also reduces the size of your iterator object.
â Martin York
Apr 16 at 22:09
 |Â
show 1 more comment
1
makeConstArrayIterator
with direction forward is missing aconst
on the 1st template argument for the return type.
â Darhuuk
Apr 16 at 13:31
@Darhuuk Thanks, fixed it.
â Timo
Apr 16 at 13:37
1
This seems an oddly specific template. A pointer-into-array is aRandomAccessIterator
. I would generalise this to adapt anyRandomAccessIterator
â Caleth
Apr 16 at 17:44
@Caleth That is indeed a good point.
â Timo
Apr 16 at 18:56
Why is units a constructor parameter and not a template parameter. You mention that pointers with different units are not comparable. So why not make them distinct types by putting the size into the template. It also reduces the size of your iterator object.
â Martin York
Apr 16 at 22:09
1
1
makeConstArrayIterator
with direction forward is missing a const
on the 1st template argument for the return type.â Darhuuk
Apr 16 at 13:31
makeConstArrayIterator
with direction forward is missing a const
on the 1st template argument for the return type.â Darhuuk
Apr 16 at 13:31
@Darhuuk Thanks, fixed it.
â Timo
Apr 16 at 13:37
@Darhuuk Thanks, fixed it.
â Timo
Apr 16 at 13:37
1
1
This seems an oddly specific template. A pointer-into-array is a
RandomAccessIterator
. I would generalise this to adapt any RandomAccessIterator
â Caleth
Apr 16 at 17:44
This seems an oddly specific template. A pointer-into-array is a
RandomAccessIterator
. I would generalise this to adapt any RandomAccessIterator
â Caleth
Apr 16 at 17:44
@Caleth That is indeed a good point.
â Timo
Apr 16 at 18:56
@Caleth That is indeed a good point.
â Timo
Apr 16 at 18:56
Why is units a constructor parameter and not a template parameter. You mention that pointers with different units are not comparable. So why not make them distinct types by putting the size into the template. It also reduces the size of your iterator object.
â Martin York
Apr 16 at 22:09
Why is units a constructor parameter and not a template parameter. You mention that pointers with different units are not comparable. So why not make them distinct types by putting the size into the template. It also reduces the size of your iterator object.
â Martin York
Apr 16 at 22:09
 |Â
show 1 more comment
2 Answers
2
active
oldest
votes
up vote
8
down vote
accepted
Design Issues
If there is one thing that bothers me about your code, it's that Reverse
template parameter. First off all, why would you code that into your class if you could just use std::reverse_iterator
and be done?
Secondly, I would argue against adding additional functionality to iterators. After all, the reason for having iterators in the first place is to have a uniform, pointer-like access to the contents of a collection. Adding additional functionality into this interface violates the principle of minimalism that the design is based on.
Other Things
- Fix your includes. You're missing
#include <type_traits>
(forstd::remove_const_t
),#include <stdexcept>
(forstd::logic_error
) and#include <cstddef>
or similar (forstd::size_t
). - Don't rely on C legacy types being defined in the global namespace.
::size_t
may not exist; usestd::size_t
instead. using difference_type = long long;
should beusing difference_type = std::ptrdiff_t
. The latter is arguably more correct here and expresses your intents better.- Why do you take an
int
as argument foroperator
? This limits your dereference to the size ofint
, whereas the underlying container can have a size ofstd::size_t
. - It appears you have typo'ed "difference" in
throw std::logic_error("Cannot calculate differnce for iterators with different unit.");
.
I completely forgot about std::reverse_iterator. Thanks for pointing that out. About your second design issue: what additional functionality are you referring to? The only overhead I can see is getUnit(), which I thought would be helpful if it's accessible from outside. Other Things: 3: I thought I read on cppreference that std::ptrdiff_t is marked as deprecated in C++17, but looks like I messed that up. I will fix the other points asap.
â Timo
Apr 16 at 14:51
@Timo I was referring to thereverse_iterator
issue with my second design point as well, so I guess you don't need to consider that anymore.
â Ben Steffan
Apr 16 at 15:01
Alright. I think I fixed everything. Can you confirm, please?
â Timo
Apr 16 at 15:06
@Timo I'm sorry, but you will have to roll back your edit. The validity and usefulness of answers on Code Review depends on code in questions not being edited after there is at least one review. If you still have doubts about your code, please post a follow up question instead.
â Ben Steffan
Apr 16 at 15:13
1
@yuri, for standard library it is not hard to remember (may be if you're working on a project with bizillion includes). There are off course edge cases, which for me is multithreading stuff.
â Incomputable
Apr 16 at 16:25
 |Â
show 3 more comments
up vote
0
down vote
Take a look at Boost.Iterator.
You can write what you want by a simple use of BoostâÂÂs iterator_adaptor
, changing only what you need to change (e.g. advance
member). Seriously, it does all that code you wrote, already in the can and well reviewed for edge cases. You only have to write whatâÂÂs different from the underlying iterator, and everything else comes through.
Or, if youâÂÂre living on the bleeding edge, just use a view::stride
in the Range v3 library.
Given a source range and an integral stride value, return a range consisting of every Nth element, starting with the first.
add a comment |Â
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
8
down vote
accepted
Design Issues
If there is one thing that bothers me about your code, it's that Reverse
template parameter. First off all, why would you code that into your class if you could just use std::reverse_iterator
and be done?
Secondly, I would argue against adding additional functionality to iterators. After all, the reason for having iterators in the first place is to have a uniform, pointer-like access to the contents of a collection. Adding additional functionality into this interface violates the principle of minimalism that the design is based on.
Other Things
- Fix your includes. You're missing
#include <type_traits>
(forstd::remove_const_t
),#include <stdexcept>
(forstd::logic_error
) and#include <cstddef>
or similar (forstd::size_t
). - Don't rely on C legacy types being defined in the global namespace.
::size_t
may not exist; usestd::size_t
instead. using difference_type = long long;
should beusing difference_type = std::ptrdiff_t
. The latter is arguably more correct here and expresses your intents better.- Why do you take an
int
as argument foroperator
? This limits your dereference to the size ofint
, whereas the underlying container can have a size ofstd::size_t
. - It appears you have typo'ed "difference" in
throw std::logic_error("Cannot calculate differnce for iterators with different unit.");
.
I completely forgot about std::reverse_iterator. Thanks for pointing that out. About your second design issue: what additional functionality are you referring to? The only overhead I can see is getUnit(), which I thought would be helpful if it's accessible from outside. Other Things: 3: I thought I read on cppreference that std::ptrdiff_t is marked as deprecated in C++17, but looks like I messed that up. I will fix the other points asap.
â Timo
Apr 16 at 14:51
@Timo I was referring to thereverse_iterator
issue with my second design point as well, so I guess you don't need to consider that anymore.
â Ben Steffan
Apr 16 at 15:01
Alright. I think I fixed everything. Can you confirm, please?
â Timo
Apr 16 at 15:06
@Timo I'm sorry, but you will have to roll back your edit. The validity and usefulness of answers on Code Review depends on code in questions not being edited after there is at least one review. If you still have doubts about your code, please post a follow up question instead.
â Ben Steffan
Apr 16 at 15:13
1
@yuri, for standard library it is not hard to remember (may be if you're working on a project with bizillion includes). There are off course edge cases, which for me is multithreading stuff.
â Incomputable
Apr 16 at 16:25
 |Â
show 3 more comments
up vote
8
down vote
accepted
Design Issues
If there is one thing that bothers me about your code, it's that Reverse
template parameter. First off all, why would you code that into your class if you could just use std::reverse_iterator
and be done?
Secondly, I would argue against adding additional functionality to iterators. After all, the reason for having iterators in the first place is to have a uniform, pointer-like access to the contents of a collection. Adding additional functionality into this interface violates the principle of minimalism that the design is based on.
Other Things
- Fix your includes. You're missing
#include <type_traits>
(forstd::remove_const_t
),#include <stdexcept>
(forstd::logic_error
) and#include <cstddef>
or similar (forstd::size_t
). - Don't rely on C legacy types being defined in the global namespace.
::size_t
may not exist; usestd::size_t
instead. using difference_type = long long;
should beusing difference_type = std::ptrdiff_t
. The latter is arguably more correct here and expresses your intents better.- Why do you take an
int
as argument foroperator
? This limits your dereference to the size ofint
, whereas the underlying container can have a size ofstd::size_t
. - It appears you have typo'ed "difference" in
throw std::logic_error("Cannot calculate differnce for iterators with different unit.");
.
I completely forgot about std::reverse_iterator. Thanks for pointing that out. About your second design issue: what additional functionality are you referring to? The only overhead I can see is getUnit(), which I thought would be helpful if it's accessible from outside. Other Things: 3: I thought I read on cppreference that std::ptrdiff_t is marked as deprecated in C++17, but looks like I messed that up. I will fix the other points asap.
â Timo
Apr 16 at 14:51
@Timo I was referring to thereverse_iterator
issue with my second design point as well, so I guess you don't need to consider that anymore.
â Ben Steffan
Apr 16 at 15:01
Alright. I think I fixed everything. Can you confirm, please?
â Timo
Apr 16 at 15:06
@Timo I'm sorry, but you will have to roll back your edit. The validity and usefulness of answers on Code Review depends on code in questions not being edited after there is at least one review. If you still have doubts about your code, please post a follow up question instead.
â Ben Steffan
Apr 16 at 15:13
1
@yuri, for standard library it is not hard to remember (may be if you're working on a project with bizillion includes). There are off course edge cases, which for me is multithreading stuff.
â Incomputable
Apr 16 at 16:25
 |Â
show 3 more comments
up vote
8
down vote
accepted
up vote
8
down vote
accepted
Design Issues
If there is one thing that bothers me about your code, it's that Reverse
template parameter. First off all, why would you code that into your class if you could just use std::reverse_iterator
and be done?
Secondly, I would argue against adding additional functionality to iterators. After all, the reason for having iterators in the first place is to have a uniform, pointer-like access to the contents of a collection. Adding additional functionality into this interface violates the principle of minimalism that the design is based on.
Other Things
- Fix your includes. You're missing
#include <type_traits>
(forstd::remove_const_t
),#include <stdexcept>
(forstd::logic_error
) and#include <cstddef>
or similar (forstd::size_t
). - Don't rely on C legacy types being defined in the global namespace.
::size_t
may not exist; usestd::size_t
instead. using difference_type = long long;
should beusing difference_type = std::ptrdiff_t
. The latter is arguably more correct here and expresses your intents better.- Why do you take an
int
as argument foroperator
? This limits your dereference to the size ofint
, whereas the underlying container can have a size ofstd::size_t
. - It appears you have typo'ed "difference" in
throw std::logic_error("Cannot calculate differnce for iterators with different unit.");
.
Design Issues
If there is one thing that bothers me about your code, it's that Reverse
template parameter. First off all, why would you code that into your class if you could just use std::reverse_iterator
and be done?
Secondly, I would argue against adding additional functionality to iterators. After all, the reason for having iterators in the first place is to have a uniform, pointer-like access to the contents of a collection. Adding additional functionality into this interface violates the principle of minimalism that the design is based on.
Other Things
- Fix your includes. You're missing
#include <type_traits>
(forstd::remove_const_t
),#include <stdexcept>
(forstd::logic_error
) and#include <cstddef>
or similar (forstd::size_t
). - Don't rely on C legacy types being defined in the global namespace.
::size_t
may not exist; usestd::size_t
instead. using difference_type = long long;
should beusing difference_type = std::ptrdiff_t
. The latter is arguably more correct here and expresses your intents better.- Why do you take an
int
as argument foroperator
? This limits your dereference to the size ofint
, whereas the underlying container can have a size ofstd::size_t
. - It appears you have typo'ed "difference" in
throw std::logic_error("Cannot calculate differnce for iterators with different unit.");
.
answered Apr 16 at 14:35
Ben Steffan
4,85011234
4,85011234
I completely forgot about std::reverse_iterator. Thanks for pointing that out. About your second design issue: what additional functionality are you referring to? The only overhead I can see is getUnit(), which I thought would be helpful if it's accessible from outside. Other Things: 3: I thought I read on cppreference that std::ptrdiff_t is marked as deprecated in C++17, but looks like I messed that up. I will fix the other points asap.
â Timo
Apr 16 at 14:51
@Timo I was referring to thereverse_iterator
issue with my second design point as well, so I guess you don't need to consider that anymore.
â Ben Steffan
Apr 16 at 15:01
Alright. I think I fixed everything. Can you confirm, please?
â Timo
Apr 16 at 15:06
@Timo I'm sorry, but you will have to roll back your edit. The validity and usefulness of answers on Code Review depends on code in questions not being edited after there is at least one review. If you still have doubts about your code, please post a follow up question instead.
â Ben Steffan
Apr 16 at 15:13
1
@yuri, for standard library it is not hard to remember (may be if you're working on a project with bizillion includes). There are off course edge cases, which for me is multithreading stuff.
â Incomputable
Apr 16 at 16:25
 |Â
show 3 more comments
I completely forgot about std::reverse_iterator. Thanks for pointing that out. About your second design issue: what additional functionality are you referring to? The only overhead I can see is getUnit(), which I thought would be helpful if it's accessible from outside. Other Things: 3: I thought I read on cppreference that std::ptrdiff_t is marked as deprecated in C++17, but looks like I messed that up. I will fix the other points asap.
â Timo
Apr 16 at 14:51
@Timo I was referring to thereverse_iterator
issue with my second design point as well, so I guess you don't need to consider that anymore.
â Ben Steffan
Apr 16 at 15:01
Alright. I think I fixed everything. Can you confirm, please?
â Timo
Apr 16 at 15:06
@Timo I'm sorry, but you will have to roll back your edit. The validity and usefulness of answers on Code Review depends on code in questions not being edited after there is at least one review. If you still have doubts about your code, please post a follow up question instead.
â Ben Steffan
Apr 16 at 15:13
1
@yuri, for standard library it is not hard to remember (may be if you're working on a project with bizillion includes). There are off course edge cases, which for me is multithreading stuff.
â Incomputable
Apr 16 at 16:25
I completely forgot about std::reverse_iterator. Thanks for pointing that out. About your second design issue: what additional functionality are you referring to? The only overhead I can see is getUnit(), which I thought would be helpful if it's accessible from outside. Other Things: 3: I thought I read on cppreference that std::ptrdiff_t is marked as deprecated in C++17, but looks like I messed that up. I will fix the other points asap.
â Timo
Apr 16 at 14:51
I completely forgot about std::reverse_iterator. Thanks for pointing that out. About your second design issue: what additional functionality are you referring to? The only overhead I can see is getUnit(), which I thought would be helpful if it's accessible from outside. Other Things: 3: I thought I read on cppreference that std::ptrdiff_t is marked as deprecated in C++17, but looks like I messed that up. I will fix the other points asap.
â Timo
Apr 16 at 14:51
@Timo I was referring to the
reverse_iterator
issue with my second design point as well, so I guess you don't need to consider that anymore.â Ben Steffan
Apr 16 at 15:01
@Timo I was referring to the
reverse_iterator
issue with my second design point as well, so I guess you don't need to consider that anymore.â Ben Steffan
Apr 16 at 15:01
Alright. I think I fixed everything. Can you confirm, please?
â Timo
Apr 16 at 15:06
Alright. I think I fixed everything. Can you confirm, please?
â Timo
Apr 16 at 15:06
@Timo I'm sorry, but you will have to roll back your edit. The validity and usefulness of answers on Code Review depends on code in questions not being edited after there is at least one review. If you still have doubts about your code, please post a follow up question instead.
â Ben Steffan
Apr 16 at 15:13
@Timo I'm sorry, but you will have to roll back your edit. The validity and usefulness of answers on Code Review depends on code in questions not being edited after there is at least one review. If you still have doubts about your code, please post a follow up question instead.
â Ben Steffan
Apr 16 at 15:13
1
1
@yuri, for standard library it is not hard to remember (may be if you're working on a project with bizillion includes). There are off course edge cases, which for me is multithreading stuff.
â Incomputable
Apr 16 at 16:25
@yuri, for standard library it is not hard to remember (may be if you're working on a project with bizillion includes). There are off course edge cases, which for me is multithreading stuff.
â Incomputable
Apr 16 at 16:25
 |Â
show 3 more comments
up vote
0
down vote
Take a look at Boost.Iterator.
You can write what you want by a simple use of BoostâÂÂs iterator_adaptor
, changing only what you need to change (e.g. advance
member). Seriously, it does all that code you wrote, already in the can and well reviewed for edge cases. You only have to write whatâÂÂs different from the underlying iterator, and everything else comes through.
Or, if youâÂÂre living on the bleeding edge, just use a view::stride
in the Range v3 library.
Given a source range and an integral stride value, return a range consisting of every Nth element, starting with the first.
add a comment |Â
up vote
0
down vote
Take a look at Boost.Iterator.
You can write what you want by a simple use of BoostâÂÂs iterator_adaptor
, changing only what you need to change (e.g. advance
member). Seriously, it does all that code you wrote, already in the can and well reviewed for edge cases. You only have to write whatâÂÂs different from the underlying iterator, and everything else comes through.
Or, if youâÂÂre living on the bleeding edge, just use a view::stride
in the Range v3 library.
Given a source range and an integral stride value, return a range consisting of every Nth element, starting with the first.
add a comment |Â
up vote
0
down vote
up vote
0
down vote
Take a look at Boost.Iterator.
You can write what you want by a simple use of BoostâÂÂs iterator_adaptor
, changing only what you need to change (e.g. advance
member). Seriously, it does all that code you wrote, already in the can and well reviewed for edge cases. You only have to write whatâÂÂs different from the underlying iterator, and everything else comes through.
Or, if youâÂÂre living on the bleeding edge, just use a view::stride
in the Range v3 library.
Given a source range and an integral stride value, return a range consisting of every Nth element, starting with the first.
Take a look at Boost.Iterator.
You can write what you want by a simple use of BoostâÂÂs iterator_adaptor
, changing only what you need to change (e.g. advance
member). Seriously, it does all that code you wrote, already in the can and well reviewed for edge cases. You only have to write whatâÂÂs different from the underlying iterator, and everything else comes through.
Or, if youâÂÂre living on the bleeding edge, just use a view::stride
in the Range v3 library.
Given a source range and an integral stride value, return a range consisting of every Nth element, starting with the first.
answered Apr 17 at 8:42
JDÃ Âugosz
5,047731
5,047731
add a comment |Â
add a comment |Â
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f192195%2fcustom-c-random-access-iterator-for-pointers-to-arrays%23new-answer', 'question_page');
);
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
1
makeConstArrayIterator
with direction forward is missing aconst
on the 1st template argument for the return type.â Darhuuk
Apr 16 at 13:31
@Darhuuk Thanks, fixed it.
â Timo
Apr 16 at 13:37
1
This seems an oddly specific template. A pointer-into-array is a
RandomAccessIterator
. I would generalise this to adapt anyRandomAccessIterator
â Caleth
Apr 16 at 17:44
@Caleth That is indeed a good point.
â Timo
Apr 16 at 18:56
Why is units a constructor parameter and not a template parameter. You mention that pointers with different units are not comparable. So why not make them distinct types by putting the size into the template. It also reduces the size of your iterator object.
â Martin York
Apr 16 at 22:09