Simple grid for simple games

Question

As my first project in C (and also final project for CS50) I made a Tetris clone using the library ncurses.

This is the grid I wrote for the game. I guess it could be easily reused for similar projects since I tried to make it "not Tetris specific".

grid.h

#ifndef GRID_HEADER
#define GRID_HEADER

#include "colors.h"
#include "helpers.h"

#define EMPTY_CONTENT 0

/*
 * grid_block:  represents a grid block.
 *
 * content: content of the block. If equal to EMPTY_CONTENT, the block is
 *          considered empty.
 * color  : color of the content.
 */
typedef struct
{
        char content;
        colors color;
} grid_block;

/*
 * grid:  represents a grid.
 *
 * blocks: a "dynamic multidimensional array" storing rows x cols blocks.
 * rows  : number of rows of the grid.
 * cols  : number of columns of the grid.
 */
typedef struct
{
        grid_block *blocks;
        int rows;
        int cols;
} grid;

/*
 * create_grid:  creates and return (a pointer to) a new empty grid of size
 *               rows x cols.
 */
grid *create_grid(int rows, int cols);

/*
 * is_valid_position:  return true if row, col is a valid position on the grid
 *                     grid.
 */
bool is_valid_position(const grid *grid, int row, int col);

/*
 * is_empty_position:  return true if the block at position row, col is empty.
 */
bool is_empty_position(const grid *grid, int row, int col);

/*
 * is_full_row:  return true if every block in row is not empty.
 */
bool is_full_row(const grid *grid, int row);

/* 
 * get_block:  return a copy of the block at position row, col.
 */
grid_block get_block(const grid *grid, int row, int col);

/*
 * fill_block:  puts content and color in the block at position row, col.
 */
void fill_block(grid *grid, char content, colors color, int row, int col);

/*
 * flush_block:  makes block at position row, col an empty block.
 */
void flush_block(grid *grid, int row, int col);

/*
 * flush_row:  makes all blocks in row empty blocks.
 */
void flush_row(grid *grid, int row);

/*
 * flush_grid:  makes all blocks in the grid grid empty blocks.
 */
void flush_grid(grid *grid);

/*
 * swap_rows:  swaps the blocks in row1 with the blocks in row2.
 */
void swap_rows(grid *grid, int row1, int row2);

/*
 * free_grid:  frees all memory occupied by grid.
 */
void free_grid(grid *grid);

#endif

colors.h

#ifndef COLORS_HEADER
#define COLORS_HEADER

#include <ncurses.h>

typedef enum
{
        BLACK   = COLOR_BLACK,
        RED     = COLOR_RED,
        GREEN   = COLOR_GREEN,
        YELLOW  = COLOR_YELLOW,
        BLUE    = COLOR_BLUE,
        MAGENTA = COLOR_MAGENTA,
        CYAN    = COLOR_CYAN,
        WHITE   = COLOR_WHITE
} colors;

#endif

helpers contains helpers functions for the entire program. Here the only one used by grid: a malloc wrapper.

helpers.h

#ifndef HELPERS_HEADER
#define HELPERS_HEADER

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

/*
 * safe_malloc:  allocates size bytes and returns a void pointer to the
 *               allocated memory. On failure, prints an error end calls
 *               exit(EXIT_FAILURE).
 */
void *safe_malloc(size_t size);

#endif

helpers.c

#include "helpers.h"


void *safe_malloc(size_t size)
{
        void *ptr = malloc(size);

        if (ptr == NULL) {
                fprintf(stderr, "error: cannot allocate %zu bytes of memory\n",
                        size);
                exit(EXIT_FAILURE);
        }
        return ptr;
}

grid.c

#include "grid.h"


grid *create_grid(int rows, int cols)
{
        grid *new_grid = safe_malloc(sizeof(*new_grid));
        grid_block *new_blocks = safe_malloc(sizeof(*new_blocks) * rows * cols);

        new_grid->blocks = new_blocks;
        new_grid->rows = rows;
        new_grid->cols = cols;

        flush_grid(new_grid);

        return new_grid;
}

bool is_valid_position(const grid *grid, int row, int col)
{
        return (row >= 0 && row < grid->rows) && (col >= 0 && col < grid->cols);
}

bool is_empty_position(const grid *grid, int row, int col)
{
        return get_block(grid, row, col).content == EMPTY_CONTENT;
}

bool is_full_row(const grid *grid, int row)
{
        for (int col = 0; col < grid->cols; col++) {
                if (is_empty_position(grid, row, col)) {
                        return false;
                }
        }
        return true;
}

grid_block get_block(const grid *grid, int row, int col)
{
        return grid->blocks[row * grid->cols + col];
}

void fill_block(grid *grid, char content, colors color, int row, int col)
{
        grid->blocks[row * grid->cols + col].content = content;
        grid->blocks[row * grid->cols + col].color = color;
}

void flush_block(grid *grid, int row, int col)
{
        grid->blocks[row * grid->cols + col].content = EMPTY_CONTENT;
}

void flush_row(grid *grid, int row)
{
        for (int col = 0; col < grid->cols; col++) {
                flush_block(grid, row, col);
        }
}

void flush_grid(grid *grid)
{
        for (int row = 0; row < grid->rows; row++) {
                flush_row(grid, row);
        }
}

void swap_rows(grid *grid, int row1, int row2)
{
        grid_block temp1;
        grid_block temp2;

        for (int col = 0; col < grid->cols; col++) {

                temp1 = get_block(grid, row1, col);
                temp2 = get_block(grid, row2, col);

                fill_block(grid, temp1.content, temp1.color, row2, col);
                fill_block(grid, temp2.content, temp2.color, row1, col);
        }
}

void free_grid(grid *grid)
{
        free(grid->blocks);
        free(grid);
}

I'd like to get some feedback. My main concerns are:

• Is the use of a malloc wrapper a "good practice"? Basically I used it every time I needed memory in the program.
• Calling functions inside other functions (e.g. like flush_grid() who calls flush_row() on each row which in turn calls flush_block() on each block) could add a significant overhead? Because in this way, it looks really "clean" to me, but I'd like to hear others people thoughts.
• Last but not least: my usage of const in the functions declarations. I used it more like a way of saying "hey look: this function will leave the grid in its current state" or "hey look: this function will modify one or more block in the grid changing its state". For example, I could declare flush_block() in this way void flush_block(const grid *grid, int row, int col); since grid->blocks, grid->rows and grid->cols will remain the same, but I thought would be more "useful" the approach I used. What do you think? (Hope I have explained my point well).

Link to the full Tetris clone for the curious.

Answer 1

1. use of a malloc wrapper

Code is reasonable. The larger issue is how to handle errors. Detect them is one thing, but them what? At least the code sends a message and quits.

The function can fail a corner case: size == 0. Returning NULL in that case is not an out-of-memory.

   void *ptr = malloc(size);
   // if (ptr == NULL) {
   if (ptr == NULL && size > 0) {
     error_code();
   }

For completeness, I'd recommend safe_realloc(), safe_calloc() and safe_free() (even if it is only a simple wrapper). Code's "safe-ness" may evolve.

2. functions inside other functions

Yes there is a function calling a function calling a function, etc. that could have been handled with one function and perform a little bit faster. Yet unless this is a real performance concern, write for clarity and maintainability. Optimization efforts are best spent reducing the order of complexity O() of a task and not simple linear performance gains. Code is fine.

3. usage of const

Code's usage is good and correct. This correct usage can allow some compilers to further optimize code as well provide the self-documentation OP noted.