Re: for_each() and two-dimensional array
Comments below, but basically looks fine to me.
Eric Lilja wrote:
On 23 Maj, 22:18, John Harrison <john_androni...@hotmail.com> wrote:
TileArray_2D foo(rows, cols);
for_each(foo.begin_1d(), foo.end_1d(), foobar());
I think I will do this, but what should my end() return?
Doesn't really matter, all that matters is that if you have an iterator
that references the last element of the array, and then you increment
that iterator, it will then be equal to the iterator that end() returns.
john
Ok, here's my result. I tried to make my extremely simple matrix class
with a nested iterator class reusable, so I wrote a templated base
class (abstract) that handles everything except the allocating of the
elements in the matrix.
I don't understand this reasoning. Why can't the memory allocation go in
the same class as everything else?
Here's the resulting code:
MatrixBase.h:
#ifndef MATRIXBASE_H
#define MATRIXBASE_H
template<typename T>
class MatrixBase
{
public:
MatrixBase(int num_rows, int num_columns)
: num_rows_(num_rows), num_columns_(num_columns) {}
virtual ~MatrixBase() { /* TODO: Deallocate memory. */ }
virtual void allocate() = 0;
class MatrixIterator
{
public:
MatrixIterator()
: matrix_(0), row_(-1), column_(-1), num_rows_(-1),
num_columns_(-1) {}
friend class MatrixBase;
T * operator*()
You could drop the assumption that says you Matrix must be a matrix of
pointers. You get a bit more flexibilty that way. And I don't see
anything else that would prevent this.
You also need two versions of operator*, one const and one non-const.
Assuming you stay with pointers they would look like this
T*& operator*()
{
...
}
T* operator*() const
{
...
}
or if you switched to let your vector hold anything like this
T& operator*()
{
...
}
const T& operator*() const
{
...
}
The non-const version allows you to change elements of the vector using
the iterator.
{
if (row_ < 0 || row_ >= num_rows_ ||
column_ < 0 || column_ > num_columns_)
throw; // FIXME
return matrix_->tiles_[column_][row_];
}
MatrixIterator& operator++()
{
if (row_ == num_rows_ - 1 && column_ == num_columns_ - 1)
{
row_ = -4711; /* -4711 is a special value that denotes
end. */
column_ = -4711;
Wouldn't row_ == num_rows_ be a more natural way of saying you are at
the end?
}
else if (column_ == num_columns_ - 1)
{
++row_;
column_ = 0;
}
else
{
++column_;
}
return *this;
}
bool operator!=(const MatrixBase::MatrixIterator& rhs)
Show be
bool operator!=(const MatrixBase::MatrixIterator& rhs) const
{
return
matrix_ != rhs.matrix_ || row_ != rhs.row_ ||
column_ != rhs.column_;
}
You also need
bool operator==(const MatrixBase::MatrixIterator& rhs) const
plus various other things.
Look up iterator_traits in Josuttis, you iterator isn't a fully fledged
iterator until you've fulfilled all the iterator requirements.
private:
MatrixIterator(MatrixBase *matrix, int num_rows,
int num_columns, int row = 0, int column = 0)
: matrix_(matrix), num_rows_(num_rows),
num_columns_(num_columns), row_(row), column_(column) {}
MatrixBase *matrix_;
int num_rows_;
int num_columns_;
The itrator class has a pointer to the matrix class, so it could get
these values from the matrix class. It's definitely good to make
iterators as small as possible since they get copied frequently.
int row_;
int column_;
};
MatrixIterator begin()
{
return MatrixIterator(this, num_rows_, num_columns_);
}
MatrixIterator end()
{
return MatrixIterator(this, num_rows_, num_columns_, -4711,
-4711);
}
protected:
T ***tiles_;
int num_rows_;
int num_columns_;
};
#endif
TileMatrix.h:
#ifndef TILEMATRIX_H
#define TILEMATRIX_H
#include "MatrixBase.h"
#include <cassert>
#include <iostream>
class Tile
{
public:
Tile(int x, int y) : x_(x), y_(y) {}
friend std::ostream& operator<<(std::ostream& os, const Tile& rhs)
{
os << "(x, y) = (" << rhs.x_ << ", " << rhs.y_ << ")";
return os;
}
private:
int x_, y_;
};
class TileMatrix : public MatrixBase<Tile>
{
public:
TileMatrix(int rows, int columns) : MatrixBase<Tile>(rows, columns)
{ allocate(); }
virtual ~TileMatrix() {}
virtual void allocate()
{
assert(num_rows_ > 0 && num_columns_ > 0);
tiles_ = new Tile**[num_rows_];
This allocation
for (int y = 0; y < num_rows_; ++y)
{
tiles_[y] = new Tile*[num_columns_];
and this allocation, could go in the base class, I think.
for (int x = 0; x < num_columns_; ++x)
{
tiles_[y][x] = new Tile(x, y);
}
}
}
};
#endif
Good job.
john