Snake Game in C++ with OOP approach

I wrote a Snake Game in C++ and I have practiced some OOP approaches. I would really appreciate criticism and some advice.

snakeGame.h

```
#ifndef _SNAKEGAME_H
#define _SNAKEGAME_H
#include
#include
#include

#define maxSnakeLenght 100

class SnakeGame{
public:
SnakeGame(int size=16, int width=25, int height=25, int timeDelay=60000)
:size(size), width(width), height(height),
widthWindow(size*width),
heightWindow(size*height),
timeDelay(timeDelay)
{}

void mainLogic();

private:
int size, width, height, timeDelay;
std::size_t widthWindow, heightWindow,sizeOfSnake=3;
class KeyboardEvents;
std::map> mapOfEvents;

sf::RenderWindow window;
sf::Texture texRed,texGreen;
sf::Sprite redFoodSprite,greenSnakeSprite;
sf::Keyboard::Key mainEvent=sf::Keyboard::Down; // always correct keyboard code
sf::Keyboard::Key codeFromKeyboard;
sf::Event event;

struct Snake{ int xCoor,yCoor; }snake[100];
struct Food{ int xCoor=10,yCoor=10; }food;

//functions
//
//bind with snake actions
void snakeMovements();
bool snakeAteFood();
bool enmeshingSnake(); // when snake will enmeshing itself, it's size will be reduced
void moveSnakeForward();
void turnSnakeUp();
void makeNewFood();
bool selfCollision(int);
bool collisionSnakeWithFood();
void snakeOutOfFrame(); // change coordinates when snake is beyond of frame
void snakeSpeed(int);
void delayTimeDecrease();
void updateEvents();
void updateMovements();

//bind with frame
void drawScreen(sf::Sprite&,sf::Sprite&,sf::RenderWindow&);
void draw();
void loadImage(const std::string&,sf::Texture&,sf::Sprite&);
void initWindow();

//bind with keyboard
void chooseMethodByKeyboard();
vo

Proper #include Guards

#ifndef _SNAKEGAME_H
#define _SNAKEGAME_H

There are two problems with this

• Names with a leading underscore are reserved for the implementation. (It's actually slightly more complicated than that but your particular use is off limits.)

• It's not very unique. I personally use NAMESPACE_PROJECT_UNIT_H_GUID if I really want to be sure to avoid any clashes.

Use Macros with caution

#define maxSnakeLenght 100

Don't use macros to define global constants. My IDE actually prompted me to change this. Use constexpr for this purpose.

SnakeGame(int size=16, int width=25, int height=25, int timeDelay=60000)
            :size(size), width(width), height(height),
            widthWindow(size*width),
            heightWindow(size*height),
            timeDelay(timeDelay)
            {}

I'm not sure how you use this because you don't provide the main() to drive the program but I doubt you need this and it will be hard to use properly. If you want to initialize the variables with defaults just do so. Do you allow the user to override those parameters on the command line before you generate the window?

Don't declare multiple variables per line

int size, width, height, timeDelay;
std::size_t widthWindow, heightWindow, sizeOfSnake=3;

Don't declare multiple variables per line. It's harder to read. It can get messy with type specifiers. And it can be confusing with assignments.

Define helper classes so you don't have to forward declare them.

class KeyboardEvents;
std::map> mapOfEvents;

If you have a nested helper class, declare it at the top rather than forward declare it in the middle of your private member variables.

Speaking of your KeyboardEvents classes, you never override on the function in the derived classes.

Don't use God Objects.

As was suggested in a comment you should break this down to a few different classes.

You probably want a Snake class. Put the size, location, speed, movement and reaction to food in there. Create a Food class that has a location and a spawn method. Then add collision detection to the application level. Have another class that wraps all the inputs. Yet another class responsible for the presentation logic. (This is known as separation of concerns.)

This will also help you better organize your code, which right now is only partially organized. Some groups of code are group together in a logical way and others seem to be distributed at random.

Don't use the C style Rand

C++'s ` is better for what you want and easier to use. I'm not entirely certain how to use C rand, because it is hard to use, but I am certain you are not getting a normal distribution. The C++ version is also prefixed with std::, as is time and srand, and it is located in the header .

Don't include twice

You included map in the .h file. You did not need to then include it in the .cpp file.

Don't use textures for solid colors

Textures and Sprites aren't meant to be used to create one solid color. sf::Color is way more lightweight for displaying red and green.

Write portable code

unistd.h is a non-portable header. The standard has a sleep call. std::this_thread::sleep_for() will provide the functionality you want cross-platform. (SFML also has sleep.)

All of this is actually skirting the fact that you are using sleep to constrain the speed of your snake. Your snake should have a velocity which you can then adjust to increase its speed as you ramp up the difficulty.

Use SFML to its fullest

SFML has Shape classes that would make your code a little easier to work with. The shapes themselves have collision detection built in, Can easily be colored or take a texture, and can be easily moved and sized. (Your sprites required your images to be the precise size you required, the shapes can be scaled and changed.)

Your keyboard event map ignores the simplicity of SFML's event system. All of that can be handled together in the updateEvent()` method.

StackExchange Code Review Q#142624, answer score: 4