Conway's Game of Life in C (3 mode)

Question

This is my program for the Game of life, I created it by myself with no reference or peer review so I need some comments and reviews on it. I created 3 modes:

A. User Mode: The program asks the user to enter the coordinates (x,y) where live creatures are to be placed on the board, until the user enters a negative coordinate.

B. Automatic Mode: The program randomly populates the board with creatures, each cell having a 1 in 10 chance of containing a creature.

C. Hybrid Mode: After having automatically populated the board, allow the user to modify it.

How can I improve this? And I'm also working on a function to detect "steady" state. If anyone have any idea please suggest.

/**Conway's Game of Life**/
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <conio.h>

const int WIDTH = 67;
const int HEIGHT = 47;
const int TRUE = 1;
const int FALSE = 0;

void makingboundarylines (int board[HEIGHT][WIDTH])
{
    int cols, rows;
    for (cols =0; cols <WIDTH; cols++)
    {
        board[0][cols] = '_';
        board[HEIGHT -1][cols]= '_';
    }
    for (rows =1; rows <HEIGHT; rows++)
    {
        board[rows][0] = '|';
        board[rows][WIDTH -1] = '|';
    }
}
void fillarray(int board[HEIGHT][WIDTH])
{
    int i,j;
    for(i =1; i <HEIGHT-1; i++)
        for(j=1; j <WIDTH-1; j++)
            board[i][j] = ' ';
}
void fillarrayrandomly(int board[HEIGHT][WIDTH])
{
    int i, j, num;
    srand((unsigned)time(NULL));
    for(i =1; i <HEIGHT -1; i++)
        for(j=1; j <WIDTH -1;j++)
        { 
            num = rand()%11;
            if (num == 1)
                board[i][j] = 'O';
            else board[i][j] = ' ';
        }
}
void display2Darray(int board[HEIGHT][WIDTH])
{
int rows;
int cols;
    for (rows = 0; rows <HEIGHT; rows++)
    {
        for(cols =0; cols <WIDTH; cols++)
            printf ("%3c", board[rows][cols]);
        printf ("\n");
    }
}
void countneighbors (int board[HEIGHT][WIDTH])
{
    int neighbors;
    int rows;
    int cols;
    int a, b;
    for (rows =1; rows <HEIGHT; rows++)
    {
        for (cols = 1; cols <WIDTH; cols ++)
        {
            neighbors = 0;
            if (board[rows][cols] == 'O')
            {
                for (a = -1; a <2; a++)
                {
                    for (b = -1; b <2; b++)
                        if (((rows +a) == rows) && ((cols +b) == cols))
                             neighbors = neighbors;
                        else if ((board[rows +a][cols +b] == 'O') ||(board[rows +a][cols +b] == 1) || (board[rows +a][cols +b] == 0))
                            neighbors++;
                }
                if ((neighbors == 2) || (neighbors == 3))
                    board[rows][cols] = 1; /*live*/
                else if ((neighbors < 2) || (neighbors >= 4))
                    board[rows][cols] = 0; /*die*/
            }
        }
    }
}
void checknewborns (int board[HEIGHT][WIDTH])
{
    int neighbors;
    int rows;
    int cols;
    int a, b;
    for (rows =1; rows <HEIGHT -1; rows++)
    {
        for (cols = 1; cols <WIDTH -1; cols ++)
        {
            neighbors = 0;
            if (board[rows][cols] == ' ')
            {
                for (a = -1; a <2; a++)
                {
                    for (b = -1; b <2; b++)
                        if (((rows +a) == rows) && ((cols +b) == cols))
                            neighbors = neighbors;
                        else if ((board[rows +a][cols +b] == 'O') || (board[rows +a][cols +b] == 1) || (board[rows +a][cols +b] == 0))
                            neighbors++;
                }
                if ((neighbors == 3))
                    board[rows][cols] = 2; /*newborn*/
            }
        }
    }
}
void anewgeneration (int board[HEIGHT][WIDTH])
{
    int rows;
    int cols;
    for (rows =1; rows <HEIGHT -1; rows++)
    {
        for (cols = 1; cols <WIDTH -1; cols ++)
        {
            if (board[rows][cols] == 1)
                board[rows][cols] = 'O';
            else if (board[rows][cols] == 2)
                board[rows][cols] = 'O';
            else if (board[rows][cols] == 0)
                board[rows][cols] = ' ';
        }
    }
}  
int checkforexistence (int board[HEIGHT][WIDTH])
{
    int rows;
    int cols;
    int creatures =0;
    for (rows =1; rows <HEIGHT -1; rows++)
    {
        for (cols = 1; cols <WIDTH -1; cols ++)
        {
            if (board[rows][cols] == 'O')
                creatures++;
        }  
    }
    if (creatures == 0)
        return TRUE; /*all creatures died*/
    else return FALSE; /*there are still living creatures*/
}
void playgame (int board[HEIGHT][WIDTH], int numgeneration)
{
    int i, c, check;
    for (i = 1; (i <= numgeneration) && (c != 32); i++)
    {
        system("cls");
        countneighbors(board);
        checknewborns(board);
        anewgeneration(board);
        printf ("Generation: %i\n", i);
        printf ("Hit enter to move to the next generation\n");
        display2Darray(board);
        if(kbhit())
        {
            c = getch();
            if(c == 32)
                break;
        }
        check = checkforexistence(board);
        if ((check= checkforexistence(board)) ==TRUE)
        {
            printf ("\nALL CREATURES HAVE DIED: GAME OVER");
            c = 32;
        }
    }
}
int returnnumber(int anumber)
{
    if  ((anumber >50) || (anumber ==0))
    { 
        printf ("ERROR! The number must be between 1 and 50\n");
        printf ("Please enter a valid number!: ");
        scanf ("%i", &anumber);
        returnnumber(anumber);
    }
    else
        return anumber;
}
void entercoordinates (int board[HEIGHT][WIDTH])
{
    int rows = 1;
    int cols =1;
    while ((rows >0) && (cols >0))
    {
        printf ("\nPlease enter x coordinate(a number from 1 to 30): ");
        scanf("%i", &cols);
        cols = returnnumber(cols);
        if (cols >0)
        {
            printf ("Please enter y coordinate(a number from 1 to 50): ");
            scanf("%i", &rows);
            rows = returnnumber(rows);
            if ((rows >0) && (cols >0))
                board[rows][cols] = 'O';
        }
    }
}
void creatingpatterntypes(int board[HEIGHT][WIDTH])
{
    int type;
    printf ("\nEnter <1> for creating  a BOX");
    printf ("\nEnter <2> for creating  a BEEHIVE");
    printf ("\nEnter <3> for creating  a TOAD");
    printf ("\nEnter <4> for creating  a SHIP");
    printf ("\nEnter <5> for creating  a GLIDER");
    printf ("\nEnter <6> for creating  a QUEEN BEE SHUTTLE");
    printf ("\nEnter <7> for creating  a PULSAR");
    printf ("\nEnter <8> for creating  a BLINKER");
    printf ("\nEnter <9> for creating  a PENTADECATHLON\n");
    scanf ("%i", &type);

if (type == 1)
{
    board[10][10] = 'O';
    board[10][11] = 'O';
    board[11][10] = 'O';
    board[11][11] = 'O';
}
else if (type == 9)
{
    board[15][10] = 'O';
    board[15][11] = 'O';
    board[15][12] = 'O';
    board[15][13] = 'O';
    board[15][14] = 'O';
    board[15][15] = 'O';
    board[15][16] = 'O';
    board[15][17] = 'O';
    board[15][18] = 'O';
    board[15][19] = 'O';
}
else if (type == 5)
{
    board[28][3] = 'O';
    board[27][4] = 'O';
    board[26][4] = 'O';
    board[27][5] = 'O';
    board[28][5] = 'O';
}
else if (type == 3)
{
    board[18][12] = 'O';
    board[18][13] = 'O';
    board[18][14] = 'O';
    board[19][11] = 'O';
    board[19][12] = 'O';
    board[19][13] = 'O';
}
else if (type == 6)
{
    board[20][28] = 'O';
    board[20][29] = 'O';
    board[21][30] = 'O';
    board[22][31] = 'O';
    board[23][31] = 'O';
    board[24][31] = 'O';
    board[25][30] = 'O';
    board[26][29] = 'O';
    board[26][28] = 'O';
}
else if (type == 7)
{
    board[12][14] = 'O';
    board[13][13] = 'O';
    board[13][14] = 'O';
    board[13][15] = 'O';
    board[14][13] = 'O';
    board[14][15] = 'O';
    board[15][13] = 'O';
    board[15][14] = 'O';
    board[15][15] = 'O';
    board[16][14] = 'O';
}
else if (type == 8)
{
    board[12][12] = 'O';
    board[12][13] = 'O';
    board[12][14] = 'O';
}
else if (type == 4)
{
    board[12][12] = 'O';
    board[12][13] = 'O';
    board[13][12] = 'O';
    board[14][13] = 'O';
    board[14][14] = 'O';
    board[13][14] = 'O';
}
else if (type ==2)
{
    board[12][14] = 'O';
    board[13][13] = 'O';
    board[13][15] = 'O';
    board[14][13] = 'O';
    board[15][14] = 'O';
    board[14][15] = 'O';
    }
}
 void usermode(int board[HEIGHT][WIDTH])
{
    int i, generation, c, choice;
    printf ("\nDo you want to insert a particular pattern type? <1> for yes and <0> for no: ");
    scanf ("%i", &choice);
    if (choice == 1)
    {
        creatingpatterntypes(board);
        display2Darray(board);
    }
    else
    {
        entercoordinates(board);
        display2Darray(board);
    }

    printf ("\nPlease select a number of generation: ");
    scanf ("%i", &generation);

    playgame(board, generation);
}
void automaticmode(int board[HEIGHT][WIDTH])
{
    int i, generation, c;
    fillarrayrandomly(board);
    display2Darray(board);
    printf ("\nPlease select a number of generation: ");
    scanf ("%i", &generation);

    playgame(board, generation);
}
void hybridmode (int board[HEIGHT][WIDTH])
{
    int i, generation, c;
    fillarrayrandomly(board);
    display2Darray(board);
    entercoordinates(board);
    system("cls");
    display2Darray(board);

    printf ("\nPlease select a number of generation: ");
    scanf ("%i", &generation);

    playgame(board, generation);
}

int main (void)
{
    //printf ("hello. world\n");
    int board[HEIGHT][WIDTH], mode;
    makingboundarylines(board);
    fillarray(board);

    printf ("CONWAY'S GAME OF LIFE\n\n");
    printf ("Please hit space to terminate the program, hit enter to move to the next generation\n");
    printf ("Please enter <1> for User Mode, <2> for Automatic Mode and <3> for Hybrid Mode: ");
    scanf ("%i", &mode);

if (mode == 1)
    usermode(board);
else if (mode == 2)
    automaticmode(board);
else if (mode ==3)
    hybridmode(board);

return 0;
}

Answer 1

I'll continue where @chux left off, not re-address any of the points already raised.

In general, your function names are quite descriptive. Descriptive function names make it so that you don't need as much documentation -- let the code document itself. However, in this function it is lacking:

int checkforexistence (int board[HEIGHT][WIDTH])
{
    //...
    if (creatures == 0)
        return TRUE; /*all creatures died*/
    else return FALSE; /*there are still living creatures*/
}

When I call this function: if (checkfirexistence(board)) ..., it is not clear what I am actually testing for. The TRUE/FALSE output of this function is not meaningful without the two comments, but those comments I cannot see when I call the function, so I need to find the function to find out what it does. I suggest you name this function areallcreaturesdead(). Now it is immediately clear what a TRUE return value means.

I also suggest you use camel case in these long names, as (at least for me) makes it easier to read them. checkForExistence, areAllCreaturesDead, etc.

---

In playgame() you have:

for (i = 1; (i <= numgeneration) && (c != 32); i++)
{
    //...
    check = checkforexistence(board);
    if ((check= checkforexistence(board)) ==TRUE)
    {
        printf ("\nALL CREATURES HAVE DIED: GAME OVER");
        c = 32;
    }
}

Here you have one redundant line, one redundant assignment, and one redundant comparison (if (checkforexistence(board)) should suffice). If TRUE, you print a message, and set c, then continue the loop. It is not immediately clear why the loop continues. One has to look at the loop condition, which uncharacteristically has two comparisons, to understand that setting c to 32 breaks the loop. Why not explicitly breaking the loop with a break statement? You do so a bit earlier in the same loop as well:

for (i = 1; i <= numgeneration; i++)           // simple for loop, easy to read
{
    //...
    if (areAllCreaturesDead(board))            // simple test, obvious what it means
    {
        printf ("\nALL CREATURES HAVE DIED: GAME OVER");
        break;                                 // obvious break from the loop
    }
}

The keyword here is "obvious". Everything you do should be obvious, so that you don't need to add comments to your code to explain what it does, so that you can quickly read the code to understand the structure and the logic, so that bugs don't hide but are forced in the open where you can catch them.

---

This next function is also rather unclear:

int returnnumber(int anumber)
{
    if  ((anumber >50) || (anumber ==0))
    { 
        printf ("ERROR! The number must be between 1 and 50\n");
        printf ("Please enter a valid number!: ");
        scanf ("%i", &anumber);
        returnnumber(anumber);
    }
    else
        return anumber;
}

It is called like this:

printf ("\nPlease enter x coordinate(a number from 1 to 30): ");
scanf("%i", &cols);
cols = returnnumber(cols);

Thus, you need to use scanf twice here. I would suggest a function that gets a number from the user and returns it:

int getNumber(const char* prompt) {
   int number = 0;
   printf(prompt);
   do {
      if (scanf("%i", &number) != 1) {
         exit(EXIT_FAILURE);
      }
   } while ((number > 50) || (number == 0));
   return number;
}

Which you call like this:

cols = getNumber("\nEnter x coordinate (a number from 1 to 30, negative to finish): ");

---

In function creatingpatterntypes() you have a long list of if ... else if ... else if .... This is a perfect candidate for a switch statement:

switch (type) {
   case 1:
      //...
      break;
   case 2:
      //...
      break;
   case //... 
}

Note that it would make sense also to sort the type codes, so it is easy to find a particular one if you need to modify or fix it.

The same is true in main where you switch on a mode value.

---

In main, you define two variables on one line:

int board[HEIGHT][WIDTH], mode;

I think it is much more readable to separate these out on different lines. Especially since the two variables defined are not really the same type, one is an array of arrays of ints, one is an int. This is better:

int board[HEIGHT][WIDTH];
int mode;

You could also use a typedef to avoid the repeated int [HEIGHT][WIDTH] bit in all these function calls:

typedef int Board[HEIGHT][WIDTH];
//...
Board board;

Answer 2

How can I improve this?

Good formatting, scant documentation

A comment per function would go a long way in describing the function's intent.

Avoid the trap of too much or too little documentation. Doc should convey the overall goal of the function, etc.

Avoid non-standard include files

#include <conio.h> invoke a non-standard C library include file. For maximum portability, avoid this and then code around kbhit(), etc.

Senseless assignments

Why neighbors = neighbors; vs. ;?

Unused parameters

uint8_t ctx_id and others are not used in static bool ctx_get_ip2(const char *qiact_reply, uint8_t ctx_id, struct ip_addr *ctx_ip), getch(). Why pass it?

Same with i and c in usermode() and automaticmode().

---

The above all are detected with ample warnings enable on my compiler.

Save time!!

Enable all compile warning and generate a warning free compilation - or use a better compiler.

gcc -std=c11 -O3 -g3 -pedantic -Wall -Wextra -Wconversion

---

Unmaintainable code

Recognize that the 60 lines of code is weak. Somehow the test should be coded more tersely.

 if (type == 1) {
    board[10][10] = 'O';
    board[10][11] = 'O';
    .... // 50+ lines
    board[14][13] = 'O';
    board[15][14] = 'O';
    board[14][15] = 'O';

Hint: anytime code is repetitive, consider some loop.

Avoid failed scanf()

The below is naked. Should the scan fail, the rest of code is suspect.

scanf("%i", &type);

Instead verify success with some code.

if (scanf("%i", &type) != 1) {
  puts("I'm going home now due to bad input.")'
  exit(EXIT_FAILURE);
}

Do you really want "%i" instead of "%d"?

Usage of #define is good

It well avoids the pitfalls of magic numbers.

srand()

Usually srand() should be called by calling code at its discretion (often only once), not by helper function such as fillarrayrandomly().

Separate code from test

A brighter line needs to defined from test code and the "Game of Life" code.

Code re-use is paramount in coding. Make clear what is "Game of Life" from the rest.

Avoid output dependencies

Minor: The below assumes the output is line buffered. Highly portable does not assume that. Flush before reading.

printf ("\nEnter <9> for creating  a PENTADECATHLON\n");
fflush(stdout);
scanf ("%i", &type);

\n at the end`

Rather than '\n' at the befing of output, use it at the end in creatingpatterntypes().

Little reason to not just do

printf ("\n" \ 
  "Enter <1> for creating  a BOX\n"
  "Enter <2> for creating  a BEEHIVE\n"
  ...
  "Enter <9> for creating  a PENTADECATHLON\n");

const

Functions like void display2Darray(int board[HEIGHT][WIDTH]) deserve to be void display2Darray(const int board[HEIGHT][WIDTH]) to allow greater application, some optimizations and clearer intent.

---

Overall

It is these small things that inhibit good review of the larger aspects of your code. In other words, I wish these small issues were not so prevalent so we could focus on the larger ones.