17
\$\begingroup\$

This is my version of the Snake Game. The project is not done yet, I still want to try to implement some other game mechanics like two player mode, high score leaderboard, etc. Also I would like to implement GUI if it won't be to hard.

At this point the game is already playable, that means that the basic game mechanics have been covered and because of that I would like to hear your opinion on the project. I would like to know how do you find my programming style, what are some areas that would need to improve, should I change or improve anything in the code or are there any better solutions for certain tasks, etc? Also I would like to hear some general advice that would be useful on future projects.

//SNAKE HEADER FILE
#include <vector>
#include <iostream>

using namespace std;

struct coordinates{int x,y;};
enum direction{UP,DOWN,LEFT,RIGHT};



class Snake {
private:
    vector<coordinates*> body;
public:
    Snake(int bodyX,int bodyY);
    ~Snake();
    int getX(int position);
    int getY(int position);
    void setX(int position,int x);
    void setY(int position,int y);
    int size();
    void step(int x,int y,bool& gameOver);
    void evolve(int x,int y,direction dir);
};

//SNAKE SOURCE FILE
#include "Snake.h"

Snake::Snake(int bodyX, int bodyY) {
    cout<<"Snake constructor is executed"<<endl;
    coordinates bodyXY={bodyX,bodyY};
    body.push_back(new coordinates(bodyXY));
}

Snake::~Snake()
{
    cout<<"Snake destructor is executed"<<endl;
}

int Snake::getX(int position) {
    return body[position]->x;
}

int Snake::getY(int position) {
    return body[position]->y;
}

int Snake::size() {
    return body.size();
}

void Snake::step(int x,int y,bool& gameOver)
{
    body.erase(body.begin());
    body.push_back(new coordinates({x, y}));
    for(int i=0;i<body.size()-1;i++)
    {
        if(body[i]->x==x&&body[i]->y==y)
        {
            gameOver=true;
            cout<<"=================================="<<endl;
            cout<<"            GAME OVER!"<<endl;
            cout<<"=================================="<<endl;
        }
    }
    //cout<<((gameOver)?"True":"False")<<endl;
}

void Snake::setX(int position, int x) {
    body[position]->x=x;
}

void Snake::setY(int position, int y) {
    body[position]->y=y;
}

void Snake::evolve(int x,int y,direction dir) {
    body.push_back(new coordinates({x,y}));
    for(int i=0;i<body.size()-1;i++)
    {
        switch(dir)
        {
            case LEFT:
                body[i]->x++;
                break;
            case RIGHT:
                body[i]->x--;
                break;
            case UP:
                body[i]->y++;
                break;
            case DOWN:
                body[i]->y--;
        }
    }
}

//APPLE HEADER
#include <cstdlib>
#include <iostream>

using namespace std;

class Apple {
private:
    int appleX,appleY;
public:
    Apple(int width,int height);
    ~Apple();
    int getX();
    int getY();
    void generate(int width,int height);

};

//APPLE SOURCE
#include "Apple.h"

Apple::Apple(int width, int height) {
    cout<<"Apple constructor is executed"<<endl;
    generate(width,height);
}

Apple::~Apple()
{
    cout<<"Apple destructor is executed"<<endl;
}

int Apple::getX() {
    return appleX;
}

int Apple::getY() {
    return appleY;
}

void Apple::generate(int width, int height) {
    appleX=rand()%(width-2)+1;
    appleY=rand()%(height-2)+1;
}

//GAME HEADER
#include "Snake.h"
#include "Apple.h"
#include <conio.h>




class Game {
private:
    int height,width;

public:
    Game(int height,int width);
    ~Game();
    void render();

};

//GAME SOURCE
#include "Game.h"

Game::Game(int height, int width) :height(height),width(width){
    cout<<"Game constructor is executed"<<endl;
}

Game::~Game()
{
    cout<<"Game destructor is executed"<<endl;
}

void Game::render()
{
    char controls;
    direction dir;
    int x,y;
    x=width/2;
    y=height/2;

    bool stop=false;
    Snake snake(x,y);
    Apple apple(width,height);
    while(!stop)
    {
        for(int i=0;i<snake.size();i++)
        {
            cout<<snake.getX(i)<<" "<<snake.getY(i)<<endl;
        }
        for(int i=0;i<height;i++)
        {
            for(int j=0;j<width;j++)
            {
                /*
                 * ============================
                 *          GAME BOARD
                 * ============================
                 * */
                if(i==0||i==height-1)
                {
                    if(j==0||j==width-1)cout<<"+";//game board corners
                    else cout<<"-";//horizontal site
                }
                else if(j==0||j==width-1)cout<<"|";//vertical site
                else
                {
                    bool print=false;
                    //IZRIS JABOLKA
                    if(apple.getX()==j&&apple.getY()==i)
                    {
                        cout<<"*";
                        print=true;
                    }
                    /*
                     * ================================
                     *         SNAKE ALGORITHM
                     * ================================
                     */
                    //if(x==j&&y==i)cout<<"X";
                    for(int k=0; k < snake.size(); k++)
                    {
                        //SNAKE PRINT
                        if(snake.getX(k)==j&&snake.getY(k)==i)
                        {
                            //HEAD
                            if(k==snake.size()-1)cout<<"X";
                            //TAIL
                            else cout<<"o";
                            print=true;
                        }
                        //BOUNDARY CONDITIONS 
                        if(snake.getX(k)>=width-1)snake.setX(k,1);
                        else if(snake.getX(k)<=0)snake.setX(k,width-2);
                        else if(snake.getY(k)>=height-1)snake.setY(k,1);
                        else if(snake.getY(k)<=0)snake.setY(k,height-2);
                        //SNAKE EATS THE APPLE
                        if(snake.getX(k)==apple.getX()&&snake.getY(k)==apple.getY())
                        {
                            apple.generate(width,height);
                            snake.evolve(x,y,dir);
                        }
                    }
                    if(!print)cout<<" ";//empty space on the board
                }
            }
            cout<<endl;
        }
        /*
         * =====================================
         *            SNAKE CONTROLS
         * =====================================
         */
        cin>>controls;
        switch (controls)
        {
            case 'a':
                x--;
                dir=LEFT;
                break;
            case 'd':
                x++;
                dir=RIGHT;
                break;
            case 'w':
                y--;
                dir=UP;
                break;
            case 's':
                y++;
                dir=DOWN;
                break;
            default:
                stop=true;
                break;
        }
        snake.step(x,y,stop);
    }
}

//AND THE MAIN SOURCE
#include <iostream>
#include "Game.h"

const int height=10;
const int width=20;

int main() {
    Game game(height,width);
    game.render();
    std::cout << "Hello, World!" << std::endl;
    return 0;
}
\$\endgroup\$

2 Answers 2

19
\$\begingroup\$

Never use using namespace std in header files

You should never write using namespace std in header files, as this can cause issues in larger projects where multiple libraries are combined. If every header file starts adding their own using namespace ..., it might result in hard to debug problems where it's no longer clear from what namespace a function or variable name is coming from.

You can safely use it in your own .cpp files, but even then I would avoid this habit. See this question for more details.

Store coordinates by value

In class Snake, you store pointers to coordinates in the vector body. But you don't need this at all, and can just store the pointers by value:

std::vector<coordinates> body;

You then no longer need to manually call new and delete to allocate memory for the coordinates. And I see you never call delete in your code, so this will already fix a memory leak.

Pass coordinates where appropriate

Since you have a nice struct coordinates, use it everywhere you have to pass coordinates instead of passing two ints, and you can also use it as a return value. For example:

Snake::Snake(coordinates position) {
    body.push_back(position);
}

...

coordinates Snake::getCoordinates(int position) {
    return body[position];
}

...

void Snake::step(coordinates position, ...) {
    body.erase(body.begin());
    body.push_back(position);
    ...
}

Use std::deque for the body coordinates

The body of the snake is added to from one end, and removed from from the other end. A std::vector is not the best container in this case, because it can only efficiently dd and remove from the back. The std::deque class does provide efficient insertion and removal from both ends, and provides easy functions for that:

class Snake {
    std::deque<coordinates> body;
    ...
};

void Snake::step(coordinates position, ...) {
    body.pop_front();
    body.push_back(position);
    ...
}

Avoid using std::endl

Prefer writing "\n" instead of std::endl. The latter is equivalent to the former, but also forces a flush of the output, which can be bad for performance. For more details, see this question.

Use range-for where appropriate

Assuming you can use C++11 features, try to use range-based for-loops where possible. For example, looping over the elements of the snake's body can be done so:

for (auto &element: body) {
    if (element.x == position.x && element.y == position.y) {
        ...
    }
}

Separate logic from presentation

Your class Snake encapsulates the logic of the snake's body, but it also prints a game over message. You should try to separate logic from presentation where possible. The function Snake::step() should just check whether the step is valid or not, and return a value indicating this. The caller can then decide whether or not to print a game over message. For example:

bool Snake::step(coordinates position) {
    body.pop_front();
    body.push_back(position);

    for (auto &element: body) {
        if (element.x == position.x && element.y == position.y) {
            return false;
        }
    }

    return true;
}

...

void Game::render() {
    ...
    while (true) {
        ...
        if (!snake.step(position)) {
            std::cout << "Game over!\n";
            break;
        }
    }
}

Use a proper random number generator

You use the C function rand(), which is a poor random number generator, but it could be good enough for a game of Snake. However, you never call srand(), which means the random number generator will always start with the same seed value, and thus always produce the same sequence of random values.

C++11 introduced much better random number generator functions. In particular, you might want to use std::uniform_int_distribution to generate integers in a given range. You could use it like so:

class Apple {
    coordinates position;

    std::mt19937 gen(std::random_device());
    std::uniform_int_distribution x_distrib;
    std::uniform_int_distribution y_distrib;

public:
    ...
    void generate();
};

void Apple::Apple(int width, int height):
    x_distrib(1, width - 1), y_distrib(1, height - 1)
{
    generate();
}

void Apple::generate() {
    position = {x_distrib(), y_distrib()};
}

Alternative way to evolve the snake body

Your method of evolving the snake's body requires saving the last direction the snake moved in, and you also move the whole body. In a typical Snake game, what happens is that the snake's body stays in the same place, but for the next move the tail will not shrink. To do this, you can keep a variable that tracks whether the snake needs to grow:

class Snake {
    std::deque<coordinates> body;
    int grow = 0;
    ...
public:
    ...
    void grow(int size);
};

void Snake::grow(int size) {
    grow += size;
}

bool Snake::step(coordinates position) {
    if (!grow) {
         body.pop_front();
    } else {
         grow--;
    }

    body.push_back(position);
    ...
};

So when the snake eats an apple, you can just call snake.grow(1). And this way, you can easily make the game harder by increasing the amount of elements the snake grows for each apple it eats.

Improve handling the snake wrapping the screen

When the snake makes a move, the only part of its body that could wrap round is its head. There is no need to check all elements of its body to see if they are out of bounds. So after reading the input, you should check whether the new head position has crossed the board boundaries, and if so wrap the coordinates. Only then call Snake::step().

Alternatively, you could just call Snake::step() with the delta position, and handle the updating of the position in Snake::step().

Split Game::render() up

The function Game::render() does too many things. It not only renders the current board, it also handles input and performs most of the game logic. I suggest you create a function named Game::loop() which just does a high level implementation of the game loop, and calls other functions that implement the various parts I just mentioned, including Game::render() which now should only render the board.

Make a function Game::handle_input() that handles the player's input. It should just make the snake do one step.

Make a function Game::logic() that implements the rest of the game logic. In particular, check if the snake's head is at the same position of the apple, or if the snake bit itself.

Doing this will make these functions small and concise, making maintenance easier.

\$\endgroup\$
3
  • 3
    \$\begingroup\$ I didn't expect such an resourcefull answer. Thank you verry much sir. Right now I am trying to implement the things you wrote. \$\endgroup\$
    – Bunny
    Aug 14, 2020 at 7:34
  • \$\begingroup\$ Very nice and formatted answer. Thank you dear @G. Sliepen. \$\endgroup\$
    – Faither
    Aug 14, 2020 at 15:36
  • 1
    \$\begingroup\$ @Bunny I wouldn't call them sir, at least, you sure about their gender, though. \$\endgroup\$
    – Faither
    Aug 14, 2020 at 15:41
1
\$\begingroup\$

Code update:

Coordinates.h

#ifndef SNAKEGAMEVOL2_COORDINATES_H
#define SNAKEGAMEVOL2_COORDINATES_H

struct coordinates{
    int x,y;
    friend bool operator==(const coordinates& l,const coordinates& r)
    {
        return l.x == r.x && l.y == r.y;
    }
};

#endif //SNAKEGAMEVOL2_COORDINATES_H

Snake.h

#include <deque>
#include <iostream>
#include "Coordinates.h"

 


class Snake {
private:
    std::deque<coordinates> body;
    int nBody;
public:
    Snake();//default constructor
    Snake(const Snake& other);//copy constructor
    Snake(coordinates init_body);//constructor
    ~Snake();
    Snake& operator=(const Snake& other);
    coordinates getCoordinates(int position);
    void setCoordinates(int position,coordinates xy);
    int size();
    void step(coordinates coord);
    void grow(int size);
};

Snake.cpp

#include "Snake.h"

Snake::Snake()
{
    std::cout<<"Snake default constructor is executed\n";
    body.push_back({0,0});
    nBody=0;
}

Snake::Snake(const Snake& other):body(other.body),nBody(other.nBody)
{
    std::cout<<"Snake copy constructor is executed\n";
}

Snake::Snake(coordinates init_body) {
    std::cout<<"Snake constructor is executed\n}";
    body.emplace_back(init_body);
    nBody=0;
}

Snake::~Snake()
{
    std::cout<<"Snake destructor is executed\n";
}

Snake & Snake::operator=(const Snake &other)= default;

coordinates Snake::getCoordinates(int position) {
    return body[position];
}

int Snake::size() {
    return body.size();
}

void Snake::step(coordinates coord)
{
    if(!nBody)
    {
        body.pop_front();
    } else{
        nBody--;
    }
    body.push_back(coord);
}

void Snake::setCoordinates(int position, coordinates xy)
{
    body[position]=xy;
}  

void Snake::grow(int size)
{
    nBody+=size;
}

Apple.h

#include <cstdlib>
#include <iostream>
#include "Coordinates.h"



class Apple {
private:
    coordinates appleCoord;
public:
    Apple();//default constructor
    Apple(coordinates dimensions);
    ~Apple();
    coordinates getCoordinates();
    void generate(coordinates dimensions);

};

Apple.cpp

 #include "Apple.h"
    
    Apple::Apple():appleCoord({0,0})
    {
        std::cout<<"Apple default constructor is executed\n";
    }
    
    Apple::Apple(coordinates dimensions) {
        std::cout<<"Apple constructor is executed\n";
        generate(dimensions);
    }
    
    Apple::~Apple()
    {
        std::cout<<"Apple destructor is executed\n";
    }
    
    coordinates Apple::getCoordinates() {
        return appleCoord;
    }
    
    
    void Apple::generate(coordinates dimensiosns) {
        appleCoord.x=rand()%(dimensiosns.x-2)+1;
        appleCoord.y=rand()%(dimensiosns.y-2)+1;
    }

Game.h

#include "Snake.h"
#include "Apple.h"
#include <conio.h>




class Game {
private:
    int height,width;
    int x,y;
    bool stop;
    Snake snake;
    Apple apple;

public:
    Game(int height, int width);
    ~Game();
    void render();
    void logic();
    void loop();
    void input();

};

Game.cpp

#include "Game.h"

Game::Game(int height, int width) : height(height), width(width) {
    std::cout<<"Game constructor is executed\n";
    x=width/2;
    y=height/2;
    stop=false;
    snake.setCoordinates(0,{x,y});
    apple.generate({width,height});
}

Game::~Game()
{
    std::cout<<"Game destructor is executed\n";
}

void Game::loop()
{
    while(!stop)
    {
        render();
        input();
        logic();
    }
}

void Game::render()
{
    coordinates xy{};
    /*for(int s=0;s<snake.size();s++)
    {
        std::cout<<snake.getCoordinates(s).x<<" "<<snake.getCoordinates(s).y<<"\n";
    }*/
     for(int i=0;i<height;i++)
     {
         for (int j = 0; j < width; j++)
         {
             xy={j,i};
             /*
             * ============================
             *          GAME BOARD
             * ============================
             * */
             if (i == 0 || i == height - 1)
             {
                 if (j == 0 || j == width - 1)std::cout << "+";//game board corners
                 else std::cout << "-";//horizontal side
             }
             else if (j == 0 || j == width - 1)std::cout << "|";//vertical side
             //APPLE
             else if (apple.getCoordinates()==xy)std::cout << "*";
             else
             {
                 /*
                 * ============================
                 *          SNAKE
                 * ============================
                 * */
                 bool print=false;
                 for(int k=0;k<snake.size();k++)
                 {
                     if(snake.getCoordinates(k)==xy)
                     {
                         //HEAD
                         if(k==snake.size()-1) std::cout<<"X";
                         //TAIL
                         else std::cout<<"o";
                         print=true;
                     }
                 }
                 //EMPTY SPACE
                 if(!print)std::cout<<" ";
             }
         }
         std::cout<<"\n";
     }
}

void Game::logic()
{
    //BOUNDARY CONDITIONS
    if(x>=width-1)x=1;
    else if(x<=0)x=width-2;
    if(y>=height-1)y=1;
    else if(y<=0)y=height-2;
    //SNAKE EATS APPLE
    coordinates head={x,y};
    if(head==apple.getCoordinates())
    {
        apple.generate({width,height});
        snake.grow(1);
    }
    for(int i=0;i<snake.size()-1;i++)
    {
        if(head==snake.getCoordinates(i)) stop=true;
    }
    snake.step({x,y});
    //std::cout<<(snake.step({x,y})?"True":"False")<<"\n";
}

void Game::input()
{
    char controls;
    std::cin>>controls;
    switch(controls)
    {
        case 'a':
            x--;
            break;
        case 'd':
            x++;
            break;
        case 'w':
            y--;
            break;
        case 's':
            y++;
            break;
        default:
            stop=true;
            break;
    }
}

I still didn't use a better random generator for the class Apple beacuse it is easier to test my code this way.

I added a default constructor to the Apple and Snake class so I can initialize them without inputing arguments to the constructor inside Game class. Also I added copy constructor and operator = to the class Snake beacuse of the three rule. Don't know if it is necessary though.

Next two tasks that I want to tackle are how to handle the game over output, beacuse the old one dosen't do the trick anymore. Maybe should I write an extra method that would I call inside the logic() method and the method would output some text in to the console? For example:

void Game::logic()
{
...
for(int i=0;i<snake.size()-1;i++)
    {
        if(head==snake.getCoordinates(i)) 
         {
            stop=true;
            gameOver()
        }
    }
}

And I want to implement some graphical interface. I read something about SDL library and am trying to implement it on some other project. Would it be a good idea to begin with? Or should I do something else?

Also I have a non programming related question. I'm a physics student who wants to get hired as C++ (or any other language) developer. I know that my programming skills are on the beginer spectrum, so I want to learn as much as possible on my own. So I would like to know how should I do that? Am I doing it the right way, so that I tackle different kind of projects or is there another way? And what kind of project do you suggest that I should do to gain the right kind of experience?

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Hi, if you have updated your code and want it reviewed, please create a new question on CodeReview instead of posting it as an answer. You can then also a link back to this review and mention that it is an improved version of an earlier review. \$\endgroup\$
    – G. Sliepen
    Aug 18, 2020 at 6:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.