3
\$\begingroup\$

I have decided to make snake game as my first project with C++ (using OOP) and I didn't follow any tutorial while doing it, now I am frustrated and need to know if it the code is good or not and what can be improved in it?

Map class header file:

#pragma once
#include <vector>
#include <iostream>
#include "Snake.h"
#include "Wall.h"
#include "Food.h"

class Map
{
public: 
    void display_map(Snake& snake, Wall& wall, Food& food);
};

Map class source file:

#include "Map.h"

void Map :: display_map(Snake& snake, Wall& wall, Food& food)
{
    system("cls");
    for (int i = 0; i < wall.get_height(); i++)
    {
        for (int j = 0; j < wall.get_width(); j++)
        {
            vector<int> temp = { j,i };
            int flag = 1;

            {
                for (auto& elm : snake.coor)
                {
                    if (elm[0] == j && elm[1] == i)
                    {
                        for (auto& elm1 : snake.body)
                        {
                            if (elm1.pos == elm)
                            {
                                cout << elm1.symbol;
                                j++;
                            }
                        }
                    }
                }
            }

            if (food.pos[0] == j && food.pos[1] == i)cout << food.symbol;
            else if ((i == 0 || i == wall.get_height() - 1) || (j == 0 || j == wall.get_width() - 2))
            {
                for (int k = 0; k < wall.wall_coor.size(); k++)
                {
                    if (wall.wall_coor[k] == temp)
                    {
                        flag = 0;
                    }

                }

                if (flag == 1)
                    wall.wall_coor.push_back(temp);

                cout << wall.get_symbol();
            }
            else cout << " ";

        }
        cout << endl;
    }

}

Snake class header file:

#pragma once
#include <vector>
#include <conio.h>

using namespace std;
class Snake
{
//Attributes
public:

    struct Body
    {
        char symbol;
        vector <int> pos;
    };
    vector <vector<int>> coor;
    vector <Body> body;
    int dir = 1;
    
//Behaviors
    void add_to_body(char ch, int x, int y);
    int change_dir();
    
    
//Constructor
    Snake(char ch = 'X', int x = 40, int y = 10);
};

Snake class source file:

#include "Snake.h"

void Snake :: add_to_body(char ch, int x, int y)
{
    Body temp;
    temp.symbol = ch;
    temp.pos = { x,y };
    coor.push_back(temp.pos);
    body.push_back(temp);
}

int Snake :: change_dir()
{
    switch (_getch())
    {
    case 'w':
        dir = 1;
        return dir;
        break;
    case 'a':
        dir = 2;
        return dir;
        break;
    case 's':
        dir = 3;
        return dir;
        break;
    case 'd':
        dir = 4;
        return dir;
        break;
    default:;

    }
}

Snake :: Snake(char ch , int x , int y)
{
    add_to_body(ch, x, y);
    add_to_body('o', x, y + 1);
    add_to_body('o', x, y + 2);
    add_to_body('o', x, y + 3);
    add_to_body('o', x, y + 4);
};

Food class header file:

#pragma once
#include <vector>
#include <ctime>

using namespace std;

class Food
{
public:
    //Attributes
    char symbol = '@';
    vector <int> pos;


    //Behavior
    vector <int> generate_food_x_y_coordinates(int x, int y);


    //Constructor
    Food(char symbol = '@', vector<int> position = { 78,4 });

};

Food class source file:

#include "Food.h"

vector <int> Food :: generate_food_x_y_coordinates(int x, int y)
{
    srand(time(NULL));
    vector <int> position = {(rand() * 2 % (x-4)) + 2   , (rand() * 2 % (y - 4)) + 2 };
    return position;
}

Food :: Food(char symbol, vector<int> position )
    :symbol{ symbol }, pos{ position } {};

Wall class header file:

#pragma once
#include <vector>

using namespace std;

class Wall
{
    char symbol;
    int width;
    int height;


public:
    vector <vector<int>> wall_coor = { {0,0} };

    int get_width()
    {
        return width;
    }
    int get_height()
    {
        return height;
    }

    char get_symbol()
    {
        return symbol;
    }

    Wall(char wall = '#', int width = 81, int height = 21)
        :symbol{ wall }, width{ width }, height{ height }{}

};

For some reason I decided to wrap all the wall class in a header file because there is not so much in the behavior of the class; only getter functions.

Finally:

The main function (game logic):

#include "Map.h"
#include "Snake.h"
#include "Wall.h"
#include "Food.h"

using namespace std;

void main()
{
    bool game_over = false;
    Snake snake ('X' , 78 , 12);
    Wall wall('#', 82, 22);
    Food food('@');
    Map map;

    while (!game_over)
    {
        if (_kbhit())
            snake.change_dir();

        switch (snake.dir)
        {
        case 1:

            while (!_kbhit())
            {

                for (int i = snake.coor.size() - 1; i >= 0; i--)
                {
                    if (i - 1 >= 0)
                        snake.coor[i] = snake.coor[i - 1];
                    else if (i == 0)
                        snake.coor[0][1]--;
                }

                for (int j = 0; j < snake.body.size(); j++)
                {
                    snake.body[j].pos = snake.coor[j];
                }

                for (int k = 0; k < wall.wall_coor.size(); k++)
                {
                    if (snake.coor[0] != wall.wall_coor[k])
                    {
                        continue;
                    }
                    else if (snake.coor[0] == wall.wall_coor[k])
                    {
                        game_over = true;
                        break;
                    }
                    break;
                }

                for (int k = 1; k < snake.coor.size(); k++)
                {
                    if (snake.coor[0] != snake.coor[k])
                    {
                        continue;
                    }
                    else if (snake.coor[0] == snake.coor[k])
                    {
                        game_over = true;
                        break;
                    }
                    break;
                }

                if (snake.coor[0] == food.pos)
                {
                    food.pos = food.generate_food_x_y_coordinates(wall.get_width(), wall.get_height() );
                    snake.add_to_body('o', snake.coor.back()[0] , snake.coor.back()[1]);
                }
                else
                {
                    food.pos = food.pos;
                }

                map.display_map(snake, wall, food);
            }

            if (snake.change_dir() == 3)
            {
                snake.dir = 1;
                continue;
            }

            break;

        case 2:
            while (!_kbhit())
            {
                for (int i = snake.coor.size() - 1; i >= 0; i--)
                {
                    if (i - 1 >= 0)
                        snake.coor[i] = snake.coor[i - 1];
                    else if (i == 0)
                        snake.coor[0][0] -= 2;
                }

                for (int j = 0; j < snake.body.size(); j++)
                {
                    snake.body[j].pos = snake.coor[j];
                }

                for (int k = 0; k < wall.wall_coor.size(); k++)
                {
                    if (snake.coor[0] != wall.wall_coor[k])
                    {
                        continue;
                    }
                    else if (snake.coor[0] == wall.wall_coor[k])
                    {
                        game_over = true;
                        break;
                    }
                    break;
                }

                for (int k = 1; k < snake.coor.size(); k++)
                {
                    if (snake.coor[0] != snake.coor[k])
                    {
                        continue;
                    }
                    else if (snake.coor[0] == snake.coor[k])
                    {
                        game_over = true;
                        break;
                    }
                    break;
                }

                if (snake.coor[0] == food.pos)
                {
                    food.pos = food.generate_food_x_y_coordinates(wall.get_width() - 4, wall.get_height() - 4);
                    snake.add_to_body('o', snake.coor.back()[0] , snake.coor.back()[1] );
                }
                else
                {
                    food.pos = food.pos;
                }

                map.display_map(snake, wall, food);
            }

            if (snake.change_dir() == 4)
            {
                snake.dir = 2;
                continue;
            }

            
            break;
        case 3:
            while (!_kbhit())
            {
                for (int i = snake.coor.size() - 1; i >= 0; i--)
                {
                    if (i - 1 >= 0)
                        snake.coor[i] = snake.coor[i - 1];
                    else if (i == 0)
                        snake.coor[i][1]++;
                }
                for (int j = 0; j < snake.body.size(); j++)
                {
                    snake.body[j].pos = snake.coor[j];
                }

                for (int k = 0; k < wall.wall_coor.size(); k++)
                {
                    if (snake.coor[0] != wall.wall_coor[k])
                    {
                        continue;
                    }
                    else if (snake.coor[0] == wall.wall_coor[k])
                    {
                        game_over = true;
                        break;
                    }
                    break;
                }

                for (int k = 1; k < snake.coor.size(); k++)
                {
                    if (snake.coor[0] != snake.coor[k])
                    {
                        continue;
                    }
                    else if (snake.coor[0] == snake.coor[k])
                    {
                        game_over = true;
                        break;
                    }
                    break;
                }

                if (snake.coor[0] == food.pos)
                {
                    food.pos = food.generate_food_x_y_coordinates(wall.get_width() - 4, wall.get_height() - 4);
                    snake.add_to_body('o', snake.coor.back()[0] , snake.coor.back()[1]);
                }
                else
                {
                    food.pos = food.pos;
                }

                map.display_map(snake, wall, food);
            }



            if (snake.change_dir() == 1)
            {
                snake.dir = 3;
                continue;
            }

            break;

        case 4:

            while (!_kbhit())
            {
                for (int i = snake.coor.size() - 1; i >= 0; i--)
                {
                    if (i - 1 >= 0)
                        snake.coor[i] = snake.coor[i - 1];
                    else if (i == 0)
                    {
                        snake.coor[i][0] += 2;
                    }
                }

                for (int j = 0; j < snake.body.size(); j++)
                {
                    snake.body[j].pos = snake.coor[j];
                }

                for (int k = 0; k < wall.wall_coor.size(); k++)
                {
                    if (snake.coor[0] != wall.wall_coor[k])
                    {
                        continue;
                    }
                    else if (snake.coor[0] == wall.wall_coor[k])
                    {
                        game_over = true;
                        break;
                    }
                    break;
                }

                for (int k = 1; k < snake.coor.size(); k++)
                {
                    if (snake.coor[0] != snake.coor[k])
                    {
                        continue;
                    }
                    else if (snake.coor[0] == snake.coor[k])
                    {
                        game_over = true;
                        break;
                    }
                    break;
                }

                if (snake.coor[0] == food.pos)
                {
                    food.pos = food.generate_food_x_y_coordinates(wall.get_width() - 4, wall.get_height() - 4);
                    snake.add_to_body('o',snake.coor.back()[0]  , snake.coor.back()[1] );
                }
                else
                {
                    food.pos = food.pos;
                }

                map.display_map(snake, wall, food);
            }

            if (snake.change_dir() == 2)
            {
                snake.dir = 4;
            }

            break;
    }
        
    }
    
    system("pause");
}
\$\endgroup\$
2
  • \$\begingroup\$ if it the code is good or not and what can be improved in it? - yes, great; we can answer that. However: re. It also have some bugs that I can't fix till now - that part is off-topic for Code Review and would need to be addressed separately on Stack Overflow. If you still want the general-purpose code review, I suggest that you edit your question. \$\endgroup\$ – Reinderien Apr 10 at 22:50
  • \$\begingroup\$ Oops, I am new to the platform, thanks for your time! \$\endgroup\$ – PixD Apr 10 at 23:12
3
\$\begingroup\$

If this is your first C++ project, then congratulations, it is not a bad attempt at all! However, there are still a lot of areas where your code can be improved. I'll list some of those below. I recommend you try to improve your code based on these suggestions, and the consider creating a new code review question here with the updated code.

Don't make unnecessary classes

I have decided to make snake game as my first project with C++ (using OOP)

I don't know if this mindset is what caused this issue, or maybe experience with Java, however not everything needs to be a class in C++. The very first piece of code you pasted in your question is this:

class Map
{
public: 
    void display_map(Snake& snake, Wall& wall, Food& food);
};

This class does not have any storage of its own, and only one public member function. That means the class is unnecessary, you can just write a stand-alone function called display_map().

In object oriented programming, objects represent some data, along with some functions to manipulate that data. Conversely, if there is no data, then it probably is not an object.

Avoid calling system()

Calling system() has several issues:

  • It is very inefficient, as it spawns a new process that runs a shell (cmd.exe on Windows), which in turn interprets the command you gave it, and if that command is not a built-in function of the shell, another process has to be spawned to actually execute that command.
  • It is not portable. cls for example only works on Windows, for Linux and macOS you would have to call clear.
  • There are security issues, since what actually happens can depend on environment variables.

To avoid having to call system(), I recommend the following:

Use a curses library

I see you want to make a game that runs in a console window. To do this in an efficient and cross-platform way, I recommend you use a curses library. There are several implementations of them, such as PDCurses and ncurses, but they all have the same basic API. They also have functions to read keyboard input.

Have a look at these code review questions for examples of a snake game using curses.

Create a struct to hold coordinates

I see you use a std::vector<int> to pass coordinates. This is very inefficient, as it is a dynamically allocated array. You could consider using a std::array<int, 2> instead, but even better is to create a struct that represents a position:

struct Position {
    int x;
    int y;
};

Use this whereever you used std::vector<int>, and even in places where you used two separate ints. For example:

class Food {
public:
    char symbol = '@';
    Position pos;

    Position generate_food_x_y_coordinates(int width, int height);
    Food(char symbol = '@', Position pos = {78, 4});
};

Use proper random number generators

Avoid C's srand() and rand() functions, C++11 introduced proper random number generators. Here is how you can modify generate_food_x_y_coordinates():

Position Food::generate_food_x_y_coordinates(int width, int height) {
    static std::random_device dev;
    static std::default_random_engine eng(dev);
    std::uniform_int_distribution<int> random_x(2, width - 2);
    std::uniform_int_distribution<int> random_y(2, height - 2);

    return {random_x(eng), random_y(eng)};
}

Avoid storing redundant information

I see that class Snake has a vector of Body elements, as well as a vector of coordinates. However, each Body element also contains its coordinates. I would remove coor and just use the positions stored in body.

Avoid repeating yourself

There is quite a lot of repetition in yuour code, in particular in the main game logic, where you repeat a lot of code for each of the four directions the snake can be moving in. Try to refactor this code. For example, in the switch (snake.dir) statement, only calculate where the new position of the snake's head will be. Afterwards, you can check whether that position would hit itself, or whether it will find some food. Also split code into multiple functions, to keep everything readable and maintainable. Ideally, the main game loop should look like:

while (!game_over) {
    process_input(...);
    game_logic(...);
    display_map(...);
}

In process_input(), you would check for keyboard input and change the direction of the snake if necessary. The function update_state() would look like:

void game_logic(Snake& snake, Wall& wall, Food& food) {
    Position next_pos = calculate_next_head_pos(snake);

    if (snake_hits_itself(snake, next_pos)) {
        game_over = true;
        return;
    }

    if (/* snake eats food */) {
        food.pos = ...;
        snake.add_to_body(...);
    }
}
\$\endgroup\$

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.