Conway's Game of Life simulation

Question

I went ahead and quickly typed this up and am unsure on how to improve. How could this be optimized? Note: there are some functions from a custom library here that do exactly what they sound like they do (e.g. make_window or fill_rectangle).

struct block{
    int a, b;

    block(){}

    block(int first, int second)
    {a = first; b = second;}

    int getVal(int x){
        if(x==1)
            return a;
        else 
            return b;
    }
};

struct squares{
    int x, y;
    int neighbors;
    int freeSpots;
    block** ary;
    int rows, cols;

    squares(int row, int col){
        //ary = new int*[row];
        ary = new block*[row];
        for(int i = 0; i < row; ++i)
            ary[i] = new block[col];
        rows = row; cols = col;

        for(int i = 0; i < row; i++){
            for(int k = 0; k < col; k++){
                ary[i][k].a=0;
                ary[i][k].b=0;
            }
        }
    }

    void clearArr(int which){
        cout<<"clear arr"<< endl;
        if(which == 0){
            for(int i = 0; i < rows; i++){
                for(int k = 0; k < cols; k++){
                    ary[i][k].a=0;
                }
            }
        }else{
            for(int i = 0; i < rows; i++){
                for(int k = 0; k < cols; k++){
                    ary[i][k].b=0;
                }
            }
        }
    }

    void placeSquares(){
        set_pen_color(color::black);
        while(1){
            wait_for_mouse_click();
            int xClick = get_click_x();
            int yClick = get_click_y();
            int x = xClick/10;
            int y = yClick/10;
            cout<< x << " " << y << endl;
            ary[x][y].a = 1;
            fill_rectangle(x*10,y*10,10,10);
            if(xClick >= 450)
                break;
        }
    }

    int neighborCount(int i, int k){
        int neighbors = 0;
            if( i>=1 && k >=1 && k<49 && i<49){
                if(ary[i+1][k].a == 1)
                    neighbors += 1;
                if(ary[i-1][k].a == 1)
                    neighbors += 1;
                if(ary[i][k+1].a == 1)
                    neighbors += 1;
                if(ary[i][k-1].a == 1)
                    neighbors += 1;
                if(ary[i+1][k+1].a == 1)
                    neighbors += 1;
                if(ary[i+1][k-1].a == 1)
                    neighbors += 1;
                if(ary[i-1][k+1].a == 1)
                    neighbors += 1;
                if(ary[i-1][k-1].a == 1)
                    neighbors += 1;
            }

        return neighbors;
    }

    void redraw(){
        while(1){
        for(int i = 0; i < rows; i++){
            for(int k = 0; k < cols; k++){
                int n = neighborCount(i,k);
                if(ary[i][k].a == 1){

                    if(n<2 || n>3)              // if less than two or more than three neighbors => death
                    {cout<< "kill"<<endl; ary[i][k].b = 0;}
                    if(n==2 || n ==3)
                        ary[i][k].b = 1;
                }else if(ary[i][k].a == 0){
                    if(n == 3)                  // if three neighbors, respawn
                        ary[i][k].b = 1;
                }
            }
        }

        // redraw
        for(int i = 0; i < rows; i++){

            for(int k = 0; k < cols; k++){
                if(ary[i][k].b == 0){
                    fill_rectangle(10*i,10*k,10,10,color::white);
                }else{
                    fill_rectangle(i*10,k*10,10,10, color::black);
                }
             }

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

        } // end while
  }
};

squares createWindow(){
    int row, col;
    cout << "Enter the number of rows followed by the number of columns desired." << endl; 
    cin >> row >> col;
    make_window(row*10,col*10);

    for(int i = 0; i < row; i++){
        move_to(0,i*10);
        draw_to(col*10,i*10);
    }

    for(int i = 0; i < col; i++){
        move_to(i*10, 0);
        draw_to(i*10,row*10);
    }

    squares sq(row, col);
    return sq;
}


void main(){

    squares sq = createWindow();
    sq.placeSquares();
        sq.redraw();

}

Answer 1

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;
      }
   }