HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Tetris code for pieces and map

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
piecesmapandforcodetetris

Problem

I'm making a simple Tetris clone and would like to know how's my code for the map and pieces. I'm using x, y, x_before, y_before, t0, t1 to allow smooth movement.

colors.h

#ifndef TETRIS_COLORS_H
#define TETRIS_COLORS_H

typedef enum {
    DARK_CYAN, DARK_RED, DARK_BROWN, DARK_MAGENTA, 
    DARK_GRAY, DARK_GREEN, DARK_BLUE, WALL, EMPTY
} Color;

#endif


pieces.h

#ifndef TETRIS_PIECES_H
#define TETRIS_PIECES_H

#include "colors.h"
#include "map.h"
#include "definitions.h"

#define PIECE_COUNT 7
#define PIECE_ROWS 4
#define PIECE_COLUMNS 4
#define PIECE_POINTS (PIECE_ROWS * PIECE_COLUMNS)
#define PIECE_BLOCKS_SIZE 4

typedef struct {
    int n;
    int matrix[4][16];
    Color color;
    int x;
    int y;
    int x_before;
    int y_before;
    unsigned int t0;
    unsigned int t1;
} Piece;

void piece_rotate(Piece *piece);
void piece_rotate_backwards(Piece *piece);
void piece_random(Piece *dest);
void piece_new(Piece *piece);
void piece_draw(Map *map, Piece *piece);
int piece_valid_position(Map *map, Piece *piece);

#endif


pieces.c

```
#include "pieces.h"
#include
#include

Piece i = {0,

0, 0, 0, 0,
1, 1, 1, 1,
0, 0, 0, 0,
0, 0, 0, 0,

0, 0, 1, 0,
0, 0, 1, 0,
0, 0, 1, 0,
0, 0, 1, 0,

0, 0, 0, 0,
0, 0, 0, 0,
1, 1, 1, 1,
0, 0, 0, 0,

0, 1, 0, 0,
0, 1, 0, 0,
0, 1, 0, 0,
0, 1, 0, 0

, DARK_CYAN, 0, 0, 0, 0, 0, 0};

Piece j = {0,

1, 0, 0, 0,
1, 1, 1, 0,
0, 0, 0, 0,
0, 0, 0, 0,

0, 1, 1, 0,
0, 1, 0, 0,
0, 1, 0, 0,
0, 0, 0, 0,

0, 0, 0, 0,
1, 1, 1, 0,
0, 0, 1, 0,
0, 0, 0, 0,

0, 1, 0, 0,
0, 1, 0, 0,
1, 1, 0, 0,
0, 0, 0, 0
, DARK_RED, 0, 0, 0, 0, 0, 0};

Piece l = {0,

0, 0, 1, 0,
1, 1, 1, 0,
0, 0, 0, 0,
0, 0, 0, 0,

0, 1, 0, 0,
0, 1, 0, 0,
0, 1, 1, 0,
0, 0, 0, 0,

0, 0, 0, 0,
1, 1, 1, 0,
1, 0, 0, 0,
0, 0, 0, 0,

1, 1, 0, 0,
0, 1, 0, 0,
0, 1, 0, 0,

Solution

Overall this looks very nice. It's easy to read, and is very refined. A few notes:

-
You can remove the Map from the first line of your structure.

typedef struct
{
    Color *blocks;
    int rows;
    int columns;
} Map;


-
Your function map_delete() only performs one action, freeing the memory of map. Unless you plan to expand on the map structure where you will have to free more members within the structure itself, I would get rid of it and just call free(map). If you have compiler optimizations on, I guarantee that your compiler is already doing that for you anyways, so it should slightly decrease compilation time.

-
You can simpilfy your NULL tests.

if(!map)


-
This is more of a style issue, but since this recently led to the Apple goto security flaw, I'm going to cover it. I don't think you are writing your brace-free single statement loops and test conditionals correctly. This is completely up to you to decide since this is a style issue, but I prefer to do it like this:

if(!map) return NULL;


Other's may be more strict and tell you to use braces, but it's up to you.

-
You need more comments. There are some lines in your code that I have trouble following. And if I have trouble following it, chances are that you will have trouble following it as well when you re-visit this project in a few months. Save yourself (and possibly others) the trouble and document your code.

Code Snippets

typedef struct
{
    Color *blocks;
    int rows;
    int columns;
} Map;
if(!map) return NULL;

Context

StackExchange Code Review Q#47754, answer score: 5

Revisions (0)

No revisions yet.