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

First small project - Snake in C

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

Problem

We started programming in school this year. I wasn't expecting to learn much but the pace is really slow so I tried to learn some things myself. I decided to do a little project for a start and that is a simple snake game. I would like to know what I am doing bad ( I know about the lack of consistency in naming ) or if I actually did something good. Some best practices I am not following and should etc.

```
#include
#include
#include
#include

#define myY 12
#define myX 24

char map[myY][myX], key;
int alive = 1, x, y, gameSpeed = 200, score = 0, dirX, dirY, oldBodyCor[myYmyX][2], body[myYmyX][2], numOfBody = 0;

void genMap();
void draw();
void move();
void genPlayer();
void genEnemy();
void hideCursor();
void kInp();
void genDefDir();
void goToXY(int column, int line);
void grow();
void freeMove();
void reset();

int main() {
hideCursor();
do {
system("cls");
genMap();
genPlayer();
genDefDir();
genEnemy();
while (alive) {
Sleep(gameSpeed);
kInp();
move();
draw();
}
system("cls");
while (1) {
goToXY(0, 0);
printf("YOU DIED");
printf("\nFINAL SCORE : %d", score);
printf("\nDo you want to try again?[y/n]");
key = _getch();
if (key == 'y') {
alive = 1;
reset();
break;
} else if (key == 'n') {
EXIT_SUCCESS;
break;
}
}
} while (alive);
}

void genMap() {
int x, y;
for (y = 0; y (myX / 2) - 1) {
if ((rand() % 2) == 0) {
dirX = -1;
dirY = 0;
return;
} else {
dirX = 0;
dirY = 1;
return;
}
} else if (y > (myY / 2) - 1 && x (myY / 2

Solution

As a complement to JS1's already good review, here are some things that may help you improve your code.

Fix the bug

The code currently includes this:

} else if (key == 'n') {
    EXIT_SUCCESS;
    break;
}


However, that middle line is not doing what you think it is. Instead, you need to write something like this:

return EXIT_SUCCESS;


Otherwise just EXIT_SUCCESS has no effect.

Use a switch instead of long if ...else chain

The key matching logic is much easier to see if a switch statement is used instead of the long if...else chain. The default case can then be used for the keys that are not valid (if you choose to do something with that).

Don't use system("cls")

There are two reasons not to use system("cls") or system("pause"). The first is that it is not portable to other operating systems which you may or may not care about now. The second is that it's a security hole, which you absolutely must care about. Specifically, if some program is defined and named cls or pause, your program will execute that program instead of what you intend, and that other program could be anything. First, isolate these into a seperate functions cls() and pause() and then modify your code to call those functions instead of system. Then rewrite the contents of those functions to do what you want using C++. For example, if your terminal supports ANSI Escape sequences, you could use this:

void cls()
{
    printf("\x1b[2J");
}

Code Snippets

} else if (key == 'n') {
    EXIT_SUCCESS;
    break;
}
return EXIT_SUCCESS;
void cls()
{
    printf("\x1b[2J");
}

Context

StackExchange Code Review Q#151213, answer score: 6

Revisions (0)

No revisions yet.