4
\$\begingroup\$

For an Information System Security course I've applied to, I need to learn C and decided to start learning it yesterday.

Conway's Game of Life has historically been a good project to start with, so I picked it. Here's a GIF of the end result (sorry about the wobble; it's just simple console printing so the result isn't perfect. I might try using Curses next):

Conway's GoL Sample

I'd like comments on anything. I've never worked this low down before, so I'm likely doing some things incorrectly. I have a few main concerns though:

  • Am I allocating and handling memory correctly? Do I have any undefined behavior?

  • Is there a better way of writing formatWorld? In Clojure, that would be like 5 lines. It ended up getting a little messy on me here though. String operations seem incredibly difficult to pull off.

  • Is my use of size_t appropriate here? It seems to be the type to use when dealing with data that shouldn't be allowed to be negative, but as soon as I started looking for places to use it, I realized that it seems it should be used everywhere.

  • Am I using headers properly? It seems like I should declare functions in the header that I expect the end user to use, and don't declare internal "private" functions.

  • Any stylistic/convention concerns.


world.h

#pragma once
#include <stdbool.h>
#include <stdlib.h>

typedef struct {
    bool* readCells;
    bool* writeCells;

    size_t width;
    size_t height;

} World;

// All write operations are done to writeArray, and all read operations are read from
//  read array.

// Returns a pointer to a newly allocated World with the given dimensions
World* newWorld(size_t width, size_t height);

// Gets/Sets the Cell at the given position
void setCell(World* world, size_t x, size_t y, bool isAlive);
bool getCell(World*, size_t x, size_t y);

// Returns a char array representing the World 
char* formatWorld(World*);
// Helper that prints the array returned from the function
void printWorld(World*);

// Advances the world by one "tick"
void advanceWorld(World*);

world.c

#include <stdio.h>
#include <stdbool.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>

#include <windows.h> // For sleep

#include "world.h"


#define MOORE_SEARCH_DEPTH 1

#define WORLD_WIDTH 50
#define WORLD_HEIGHT 50

#define LOOP_ADVANCE_DELAY 100

// Returns a representation of a non-ragged 2D array with the given dimensions
//  where all the cells are dead
bool* newDeadCells(size_t width, size_t height) {
    bool* cells = malloc(sizeof(bool) * width * height);

    for (size_t i = 0; i < (width * height); i++) {
        cells[i] = false;
    }

    return cells;
}

World* newWorld(size_t width, size_t height) {
    World* w = malloc(sizeof(World));

    w->width = width;
    w->height = height;

    w->readCells = newDeadCells(width, height);
    w->writeCells = newDeadCells(width, height);

    return w;
}

// Randomizes the cells so that each cell has an aliveChance chance of being alive
void randomizeCells(World* world, float aliveChance) {
    size_t width = world->width;
    size_t height = world->height;

    for (size_t i = 0; i < (width * height); i++) {
        world->readCells[i] = (rand() % 100) < aliveChance * 100;
    }
}

// Overwrites the readable cells with the writable cells
void copyWritableToReadable(World* world) {
    memcpy(world->readCells, world->writeCells,
        sizeof(bool) * world->width * world->height);
}

// Frees the given World and any memory associated with it
void freeWorld(World* world) {
    free(world->readCells);
    free(world->writeCells);
    free(world);
}

size_t indexOf(size_t width, size_t x, size_t y) {
    return width * y + x;
}

void setCell(World* world, size_t x, size_t y, bool isAlive) {
    size_t index = indexOf(world->width, x, y);

    world->writeCells[index] = isAlive;
}

bool getCell(World* world, size_t x, size_t y) {
    int index = indexOf(world->width, x, y);

    return world->readCells[index];
}

// Returns the number of live neighbors surrounding the given position.
// depth returns how many squares to look in each direction. 1 = Standard Moore Neighborhood.
size_t nAliveNeighborsSurrounding(World* world, size_t x, size_t y, size_t depth) {
    size_t xBound = min(x + depth + 1, world->width);
    size_t yBound = min(y + depth + 1, world->height);

    size_t aliveCount = 0;
    for (size_t ny = max(0, y - depth); ny < yBound; ny++) {
        for (size_t nx = max(0, x - depth); nx < xBound; nx++) {

            if (getCell(world, nx, ny) && !(nx == x && ny == y)) {
                aliveCount++;
            }
        }
    }

    return aliveCount;
}

bool cellShouldLive(bool isAlive, size_t nNeighbors) {
    return (isAlive && nNeighbors >= 2 && nNeighbors <= 3)
        || (!isAlive && nNeighbors == 3);
}

// Decides if a cell should live or die, and sets it accordingly
void advanceCellAt(World* world, size_t x, size_t y) {
    size_t nNeighbors = nAliveNeighborsSurrounding(world, x, y, MOORE_SEARCH_DEPTH);
    bool isAlive = getCell(world, x, y);

    setCell(world, x, y, cellShouldLive(isAlive, nNeighbors));
}

void advanceWorld(World* world) {
    size_t width = world->width;
    size_t height = world->height;

    for (size_t y = 0; y < height; y++) {
        for (size_t x = 0; x < width; x++) {
            advanceCellAt(world, x, y);
        }
    }

    copyWritableToReadable(world);
}

char* formatWorld(World* world) {
    size_t width = world->width;
    size_t height = world->height;

    size_t nCells = width * height;

    // total cells needed + extra for newlines + NL term
    size_t buffSize = sizeof(char) * nCells + height + 1;
    char* buffer = malloc(buffSize);
    buffer[buffSize - 1] = '\0';

    size_t i = 0;
    for (size_t y = 0; y < height; y++) {
        for (size_t x = 0; x < width; x++) {
            bool isAlive = getCell(world, x, y);
            char rep = isAlive ? '#' : ' ';
            buffer[i] = rep;

            i++;
        }

        buffer[i] = '\n';

        i++;
    }

    return buffer;
}

void printWorld(World* world) {
    char* formatted = formatWorld(world);
    printf("%s", formatted);
    free(formatted);
}

void simpleConsoleRoutine() {
    srand(NULL);

    World* world = newWorld(WORLD_WIDTH, WORLD_HEIGHT);
    randomizeCells(world, 0.3);

    // Leaving it with a counter so I can limit it easily later
    for (size_t i = 0; ; i++) {
        printWorld(world);
        printf("----------\n");

        advanceWorld(world);

        Sleep(LOOP_ADVANCE_DELAY);
    }

    // No need to free world?
}

int main() {
    simpleConsoleRoutine();
}
\$\endgroup\$
3
  • \$\begingroup\$ windows.h is very limiting for just sleep: en.wikipedia.org/wiki/Sleep_(system_call). I'd go with the more general POSIX, but then Windows will probably have trouble. Perhaps an #ifdef? \$\endgroup\$
    – Neil
    Apr 8, 2019 at 23:49
  • \$\begingroup\$ @Neil Ya, I wasn't able to find out a good general solution other than to do conditional #includes. I'm using it here purely for demo purposes, so I didn't think it would be a big deal. Ideally I wouldn't be using Sleep at all in a "real program". \$\endgroup\$ Apr 8, 2019 at 23:58
  • \$\begingroup\$ One solution which is kind of a hack is to #ifdef DEMO/#endif around the #include <windows.h> and Sleep(); then one can define DEMO in your compiler settings (Windows version with the Sleep) or not, and it will change the code. \$\endgroup\$
    – Neil
    Apr 9, 2019 at 2:27

1 Answer 1

3
\$\begingroup\$

If this is your first C program then all I can say is "Nice".

I modified the structure of your program slightly to make it more conventional, I created main.c which contains int main() and simpleConsoleRoutine() and world.c which has all the other functions.

Generally main() is going to process any command line arguments and do any setup, everything else will be in C program modules defined by a header files.

Suggestion, all the functions that are NOT public interfaces might be better defined as static. This reduces the name of functions in the global namespace and allows a function name to be reused in different modules as necessary.

Example:

// Overwrites the readable cells with the writable cells
static void copyWritableToReadable(World* world) {
    memcpy(world->readCells, world->writeCells,
        sizeof(bool) * world->width * world->height);
}

Header File Missing Function Declaration

It would be best to add void randomizeCells(World* world, float aliveChance); to the header file, since this function is required by simpleConsoleRoutine().

Portability

#pragma once may not be in the C standard and may not be portable, use guards instead

#ifndef WORLD_H
#define WORLD_H

...

#endif

Memory Allocation and De-allocation

// No need to free world?

In this particular instance there is no need to free world, however, if simpleConsoleRoutine() was ever called in a loop there would be a memory leak, which is generally considered a bad thing when programming in C. You might want to put void freeWorld(World* world) into the header file and replace this comment with a call to freeWorld(). In this simple case it would allow you to test and debug the code.

size_t

The use of this type is correct in this application. For memory allocation or indexing through arrays this may be the best type to use.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.