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

Sudoku Grid special purpose Iterators

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

Problem

Please have a look at these iterators which I use for my Sudoku solver. They behave slightly different from STL iterators and don't implement all functionality that would be needed to use them in a stl context. But the basic idea behind them was to clean up the code in the Sudoku program that makes heavy use of the three access patterns (row, col, block) that I implemented.

The most "important" iterator is the BlockIterator, since without that iterating over all nine fields in a block looked quite ugly. Iterating rows and columns wasn't that bad, but since I started writing the stuff I decided to create a complete set.

Some technical details:

The grid class holds an (evil) array of pointers to Field objects, that's one dimensional (I could have used a two dimensional array as well, but I often do it this way and feel quite comfortable with modulo operations). Maybe I will replace this with a vector later.

The grid class adds a few static functions to calculate offsets in the array based on row, col or block positions.

```
class Grid {
public:
Grid();
Grid(std::string s);

class Iterator {
public:
Iterator(Grid* g) : grid(g), it(0){}
Field operator(){return field;}
void operator++(){
++it;
if(it field[field_index];
}
protected:
int row_offset;
};

class ColIterator : public Iterator {
public:
ColIterator(Grid* g, int col) : Iterator(g){
col_offset = col;
field = calc_field();
}
Field* calc_field(){
int field_index = it * size + col_offset;
return grid->field[field_index];
}
protected:
int col_offset;
};

class BlockIterator : public Iterator {
public:
BlockIterator(Grid* g, int block) : Iterator(g){
block_offset = Grid::block_offset(block);
field = calc_field();
}
Field* calc_field(){
int field_index = bl

Solution

In my first quick scan through, here are some things I want to bring up:

  • If you are going to overload operators, do it the way the users of the language expect, or don't do it at all. I expect operator++ to return something, not be a void.



  • Does block_offset really need to be public?



  • On that same note, do your actual concrete implementations of the iterators need to be public, since you have methods to create them that are public? Would it make sense for anyone to ever want to create them a different way?

Context

StackExchange Code Review Q#558, answer score: 9

Revisions (0)

No revisions yet.