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

Full C++ Snake game

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

Problem

I managed to finish my approach on the legendary Snake game. You move around with the WASD keys. I would like to hear your opinion and maybe ideas on how I can improve frame rate because now it sort of flickers (but it is playable).

#include 
#include  
#include 
#include 
#include 

#define PLANEHEIGHT 20
#define PLANEWIDTH 50
#define INITIALSNAKELENGTH 3
#define FPS 5

struct SnakeSegment
{
    SnakeSegment() {};
    SnakeSegment( unsigned short x1,unsigned short y1 )
    {
        x = x1;
        y = y1;
    }
    unsigned short x;
    unsigned short y;
};

unsigned short snakeLength = INITIALSNAKELENGTH;
char lastDirection = 'd';
std::vector m_Snake;

unsigned short foodX;
unsigned short foodY;

char plane[ PLANEHEIGHT * PLANEWIDTH ];

void DisplayPlane();
void NormalisePlane();
void SnakeInit();
void FoodInit();
void Move();
void DeleteLastSnakeSegment();
bool HasEatenFood();

void WipeConsole();

bool Won();
bool Lost();

int main()
{
    srand( (unsigned int)time( NULL ) );

    NormalisePlane();
    SnakeInit();
    FoodInit();

    for ( ;; )
    {
        clock_t start;
        double time = 0.0;
        start = clock();

        while( time  PLANEWIDTH - 1 ||
        segment.y  PLANEHEIGHT - 1 ) return true; //if colides with boundries

    for( unsigned short i = 0; i < m_Snake.size() - 1; i++ ) //or with itself we return true
    {
        SnakeSegment temp = m_Snake[ i ];
        if( temp.x == segment.x && temp.y == segment.y ) return true;
    }

    return false;
}

Solution

Using constants

Rather than using #define and creating a bunch of macros like this:

#define PLANEHEIGHT 20
#define PLANEWIDTH 50
#define INITIALSNAKELENGTH 3
#define FPS 5


You should use const instead, like this:

const int PLANE_HEIGHT         = 20;
const int PLANE_WIDTH          = 50;
const int INITIAL_SNAKE_LENGTH = 3;
const int FRAMES_PER_SECOND    = 5;


Using constants rather than macros adds readability to your code, and it has all the properties of a normal variable, e.g, a size, a type, linkage, etcetera. A macro has none of those.

Refactoring SnakeSegment

I'd also define your SnakeSegment structure as a class instead, as seen below.

class SnakeSegment
{
public:
    SnakeSegment() {};
    SnakeSegment( unsigned short x1,unsigned short y1 )
    {
        x = x1;
        y = y1;
    }
    unsigned short x;
    unsigned short y;
};


But that's not all we can do, we can also remove the empty constructor, as it doesn't make much sense to have it. Our SnakeSegment class then becomes this:

class SnakeSegment
{
public:
    SnakeSegment( unsigned short x1,unsigned short y1 )
    {
        x = x1;
        y = y1;
    }
    unsigned short x;
    unsigned short y;
};


But that's still not all. There's a better way of initializing x and y in the constructor as well. Here's how the constructor would look after that:

SnakeSegment(unsigned short x, unsigned short y):
    x(x), 
    y(y)
{}


There's no need to worry about the arguments of the constructor having the same names as the fields, the x in x(x) and the y in y(y) will evaluate as the constructor arguments, rather than the fields.

In short, the SnakeSegment class, after a few other minor changes, the SnakeSegment class will become this:

class SnakeSegment
{
public:
    unsigned short x;
    unsigned short y;

    SnakeSegment(unsigned short x, unsigned short y):
        x(x),
        y(y)
    {}
};


It's also worth noting, if you ever want to add more methods to this class in the future, you should define two separate files for it, classname.cpp, and classname.h. classname.h should contain the class declaration and the function/constructor signatures inside it, and classname.cpp should include classname.h and implement the functions/constructors.

Not using rand()

rand() is a really bad way to generate random numbers, you should be using a solution more along the lines of this:

std::random_device                 randomDevice;
std::mt19937                       engine(randomDevice());
std::uniform_int_distribution distribution(low, high);

int result = distribution(engine);


Do note, you will have to add this line of code to the top of your file:

#include 


Nitpicks

Please, please do not use this "hack":

system("PAUSE");


It's not cross-platform, it's not readable, and it's insecure, if you need the program to pause before exiting, use something like this:

std::cin.get();


From looking at your code though, I think that your program should just exit, so there's no need for either of these solutions.

Finally, all these function signatures at the top of your code:

void DisplayPlane();
void NormalisePlane();
void SnakeInit();
void FoodInit();
void Move();
void DeleteLastSnakeSegment();
bool HasEatenFood();

void WipeConsole();

bool Won();
bool Lost();


These are a bit of code smell, and are not really necessary, they can be removed. (You may have to define these functions again above main.)

Code Snippets

#define PLANEHEIGHT 20
#define PLANEWIDTH 50
#define INITIALSNAKELENGTH 3
#define FPS 5
const int PLANE_HEIGHT         = 20;
const int PLANE_WIDTH          = 50;
const int INITIAL_SNAKE_LENGTH = 3;
const int FRAMES_PER_SECOND    = 5;
class SnakeSegment
{
public:
    SnakeSegment() {};
    SnakeSegment( unsigned short x1,unsigned short y1 )
    {
        x = x1;
        y = y1;
    }
    unsigned short x;
    unsigned short y;
};
class SnakeSegment
{
public:
    SnakeSegment( unsigned short x1,unsigned short y1 )
    {
        x = x1;
        y = y1;
    }
    unsigned short x;
    unsigned short y;
};
SnakeSegment(unsigned short x, unsigned short y):
    x(x), 
    y(y)
{}

Context

StackExchange Code Review Q#102261, answer score: 14

Revisions (0)

No revisions yet.