2d Grid - Iterating by Rows / Cells

Question

The feature

Here is a Grid class representing a 2d grid.
The class will get templated once it reach a satisfactory state.
At this time the cells are int values.

This Grid class allow for iteration by rows using simple range-for syntax.

    Grid grid(8, 8);

    for (auto&& row_it : grid.Rows())
    {
        for (auto&& cell : row_it)
            std::cout << std::setw(2) << std::setfill('0') << cell << ',';
        std::cout << '\n';
    }

The retrospective

It works well, but it is my first time creating custom iterators and I do not think that its is nicely done.

More in particular:

• Are the classes GridRows, RowsIterator and RowIterator really needed?
• The range-for will automatically call operator* on the type returned from begin(), which seems to force the existence of RowIterator instead of returning directly an int* from RowsIterator::begin
• How best to minimize and simplify?

Standard library compatibility

According to this article, custom iterators should include the following properties: https://www.internalpointers.com/post/writing-custom-iterators-modern-cpp

• iterator_category
• difference_type
• value_type
• pointer
• reference

The complete code

#include <iostream>
#include <iomanip>

struct RowIterator
{
    int* _it, * _end;

    RowIterator(int* begin, int* end) :
        _it{ begin },
        _end{ end }
    {}

    int& operator*()
    {
        return *_it;
    }

    int* operator->()
    {
        return _it;
    }

    RowIterator& operator++()
    {
        ++_it;
        return *this;
    }

    RowIterator operator++(int)
    {
        auto self = *this;
        ++* this;
        return *this;
    }

    friend bool operator==(RowIterator lhs, RowIterator rhs)
    {
        return lhs._it == rhs._it;
    }

    friend bool operator!=(RowIterator lhs, RowIterator rhs)
    {
        return lhs._it != rhs._it;
    }

    int* begin()
    {
        return _it;
    }

    int* end()
    {
        return _end;
    }
};

struct RowsIterator
{
    int* _it, * _end;
    int _row_length;
    RowIterator _row_begin, _row_end;

    RowsIterator(int* begin, int row_length) :
        _it{ begin },
        _end{ begin + row_length },
        _row_length{ row_length },
        _row_begin{ _it, _end },
        _row_end{ _end, _end }
    {}

    RowIterator& operator*()
    {
        return _row_begin;
    }

    RowIterator* operator->()
    {
        return &_row_begin;
    }

    RowsIterator& operator++()
    {
        _it += _row_length;
        _end += _row_length;
        _row_begin = { _it, _end };
        _row_end = { _end, _end };
        return *this;
    }

    RowsIterator operator++(int)
    {
        auto self = *this;
        ++* this;
        return *this;
    }

    friend bool operator==(RowsIterator lhs, RowsIterator rhs)
    {
        return lhs._it == rhs._it;
    }

    friend bool operator!=(RowsIterator lhs, RowsIterator rhs)
    {
        return lhs._it != rhs._it;
    }

    RowIterator begin() const
    {
        return _row_begin;
    }

    RowIterator end() const
    {
        return _row_end;
    }
};

struct GridRows
{
    RowsIterator _begin, _end;

    GridRows(int* begin, int* end, int row_length) :
        _begin{ begin, row_length },
        _end{ end, row_length }
    {}

    RowsIterator begin()
    {
        return _begin;
    }

    RowsIterator end()
    {
        return _end;
    }
};

struct Grid
{
    Grid(int width, int height) : _width{ width }, _height{ height }
    {
        size_t size = width * height;
        _begin = new int[size];
        _end = _begin + size;

        InitializeValues();
    }

    ~Grid()
    {
        if (_begin)
        {
            delete[] _begin;
            _begin = nullptr;
            _end = nullptr;
        }
    }

    void InitializeValues()
    {
        for (int i = 0; auto && it : *this)
            it = ++i;
    }

    int* begin() const { return _begin; }
    int* end() const { return _end; }

    GridRows Rows() const
    {
        return GridRows{ _begin, _end, _width };
    }

    int* _begin, * _end;
    int _width, _height;
};

void IterateWithFor(const Grid& grid)
{
    std::cout << "Iterate though all rows cells\n";
    auto&& rows = grid.Rows();
    for (auto&& row_it = rows.begin(); row_it != rows.end(); ++row_it)
    {
        for (auto&& cell = row_it.begin(); cell != row_it.end(); ++cell) {
            std::cout << std::setw(2) << std::setfill('0') << *cell << ',';
        }
        std::cout << '\n';
    }
    std::cout << '\n';
}

void IterateWithRangeFor(const Grid& grid)
{
    std::cout << "Iterate though all rows cells\n";
    for (auto&& row_it : grid.Rows())
    {
        for (auto&& cell : row_it)
            std::cout << std::setw(2) << std::setfill('0') << cell << ',';
        std::cout << '\n';
    }
    std::cout << '\n';
}

int main()
{
    Grid grid(8, 8);
    IterateWithFor(grid);
    IterateWithRangeFor(grid);
    return 0;
}

Thank you

Thank you very much for your time and valuable feedback.

Answer 1

RowIterator operator++(int)
{
    auto self = *this;
    ++* this;
    return *this;
}

We should be returning the previous value (self), rather than *this.

---

Grid owns storage using a raw pointer, so needs copy/move constructor and assignment operations. Or better, use a std::vector instead of int* to take care of that automatically.

---

Use std::size_t for width and height, rather than int. That way, we don't need to check that they are not negative.

---

The iterators are missing the necessary type names to be used with standard algorithms. You even mention that in the question, so fix that immediately.

---

Iterator's operator== can be defined = default, and that will also default !=.

---

RowIterator begin() const
{
    return _row_begin;
}

That's dangerous - we should be returning a const-iterator when the object is const. Also consider implementing the other (const/reverse) begin/end functions.

---

~Grid()
{
    if (_begin) …
}

I don't see any way that _begin could be false here.

---

int* begin() const { return _begin; }
int* end() const { return _end; }

Again, we need to sort out the const-correctness here.