HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Static array wrapper with iterators

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
arraywithwrapperiteratorsstatic

Problem

This is just like std::array. I wrote this for practice. The class is in a different namespace, so the name array stands!!

There is only one bug (that I'm aware of) and that's the base() function for the reverse iterators. Works fine with const_reverse_iterator, but if I do the following:

auto i = arr.rbegin();  //i is array::reverse_iterator
auto j = i.base();   //j is std::iterator


So I could use a little help with that!

Sorry about the lack of horizontal space! It looks too good in an IDE with more space!

Live example: http://coliru.stacked-crooked.com/a/e39f5760f67fa533

```
#include
#include
#include
#include
#include
#include
#include

template
class array
{
public: //TYPE ALIASES
using value_type = ArrType;
using size_type = std::size_t;
using difference_type = std::ptrdiff_t;
using reference = ArrType&;
using const_reference = const ArrType&;
using pointer = ArrType*;
using const_pointer = const ArrType*;

private:
class array_iterator : public std::iterator
{
protected:
pointer _pos;

public:
explicit constexpr array_iterator(pointer pos)
: _pos(pos)
{}

constexpr array_iterator() = default;
constexpr array_iterator(const array_iterator&) = default;
constexpr array_iterator(array_iterator&&) = default;

//ASSIGNMENT OPERATORS
array_iterator &operator=(const array_iterator&) = default;
array_iterator &operator=(array_iterator&&) = default;

//COMPARISON OPERATORS
constexpr bool operator==(const array_iterator& rhs) const { return _pos == rhs._pos; }
constexpr bool operator!=(const array_iterator& rhs) const { return _pos != rhs._pos; }
constexpr bool operator>(const array_iterator& rhs) const { return _pos > rhs._pos; }
constexpr bool operator=(const array_iterator& rhs) const { return _pos >= rhs._pos; }
constexpr

Solution

Bugs:

-
Your iterators default-constructor is bad: You need to set the member-pointer to nullptr!

-
Your iterators are missing the index-operator.

Iterators:

-
You should read about SCARY iterators. The standard-library uses them for versatility and avoidance of duplicate code:
What are SCARY iterators?

In a nutshell, your iterators should only be dependent on the type-argument, not on the length-argument.

-
There is no reason to write your own reverse-iterators.

Just use std::reverse_iterator.

-
There's also no reason to have a separate const_iterator-template once you use SCARY iterators: Just use a normal iterator with a constant value-type.

If you want to enable assignment from non-constant to constant iterators, add a free-function user-defined conversion.

-
Your template-arguments have curious names. T and N would be customary.

-
"bounds checking regardless of C++ standard": Thanks for not making it noexcept, and thus breaking everyones expectations.

Also, what do you expect the calling code to do when it gets an impossible exception?

Just std::terminate() directly on detecting such internal corruption.

-
at now should have bounds-checking and throw on out-of-bound-access. As it's more complicated than a simple field-access, you should implement the non-const version in terms of the const version, using a const_cast. Not the other way around, that would violate the standard, strictly speaking.

-
front and back should just call std::terminate if N == 0, as calling them is a bug.

-
I suggest you prefer SFINAE on the return-type instead of using an additional template-argument. That way, it isn't part of the symbol-name.

-
You are missing many opportunities for marking things noexcept and/or constexpr.

-
I wouldn't add so many algorithms as member-functions, as the free function algorithms in the standard library are perfectly adequate.

-
If a type is cheap to copy, prefer passing it by value.

Context

StackExchange Code Review Q#113081, answer score: 7

Revisions (0)

No revisions yet.