Return to Revisions

Remove unused functions and member variables

block::getValue() is not used at all.

squares::freeSpots is not used.

squares::neighbors is not used. There is a function local variable of the same name in squares::neighborCount but the class member variable is not used at all.

squares::x is not used. There is a function local variable of the same name in squares::placeSquares but the class member variable is not used at all.

squares::y is not used. There is a function local variable of the same name in squares::placeSquares but the class member variable is not used at all.

Replace hard coded numbers with suitably named constants

You have:

if( i>=1 && k >=1 && k<49 && i<49){

That would be more readable if you use:

const int ABCD_LIMIT = 49;
if( i>=1 && k >=1 && k<ABCD_LIMIT && i<ABCD_LIMIT){

Other examples of hard coded numbers:

if(xClick >= 450)

fill_rectangle(10*i,10*k,10,10,color::white);

fill_rectangle(i*10,k*10,10,10, color::black);

make_window(row*10,col*10);

move_to(0,i*10);
draw_to(col*10,i*10);

move_to(i*10, 0);
draw_to(i*10,row*10);

Replace use of pointers with std::vectors

You have

block** ary;

That can easily be

std::vector<std::vector<block>> ary;

I would also recommend changing the name ary to blockArray. A more descriptive name is better than a cryptic one.

std::vector<std::vector<block>> blockArray;

Benefits of using a std::vector<std::vector<block>>:

1. You can remove the member variables rows and cols.

2. Implementation of the constructor can be simplified to:

    squares(int row, int col) : blockArray(row, std::vector<block>(col, block)) {}
   

3. You get automatic memory cleanup. In your posted code, you don't have a user defined destructor. As a result, you have a memory leak. By using std::vectors, the default destructor provided by the compiler will take care of releasing memory used by the std::vectors.

4. It is easier to iterate over the array. You can replace the following block:

    for(int i = 0; i < rows; i++){
        for(int k = 0; k < cols; k++){
            ary[i][k].a=0;
        }
    }
   

by

    for ( auto& row : blockArray) {
       for ( auto& block : row ) {
          block.a = 0;
       }
    }