patterncMinor
Tetris code for pieces and map
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
colors.h
pieces.h
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,
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;
#endifpieces.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);
#endifpieces.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
-
Your function
-
You can simpilfy your
-
This is more of a style issue, but since this recently led to the Apple
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.
-
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.