8
\$\begingroup\$

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<SFML/Graphics.hpp>
#include<map>
#include<memory>

#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<sf::Keyboard::Key,  std::shared_ptr<KeyboardEvents>> 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();
        void chooseMethodFromKeyboardEvents(sf::Keyboard::Key);
        void initializeEventsMap();



/*********************************************************************************************************
/
                                    / EventsFromKeyboard Familly
/
/
**********************************************************************************************************/

            class KeyboardEvents{
                public:
                    virtual ~KeyboardEvents(){};
                    virtual void event(SnakeGame&)=0;
            };

            class GoRight:public KeyboardEvents{
                private:
                    void event(SnakeGame& snakeObj);
            };

            class GoLeft:public KeyboardEvents{
                private:
                    void event(SnakeGame& snakeObj);
            };

            class GoUp:public KeyboardEvents{
                private:
                    void event(SnakeGame& snakeObj);
            };

            class GoDown:public KeyboardEvents{
                private:
                    void event(SnakeGame& snakeObj);
            };
            class Exit:public KeyboardEvents{
                private:
                    void event(SnakeGame& snakeObj);
            };

            class Pause:public KeyboardEvents{ 
                private:
                    void event(SnakeGame& snakeObj);
            };


};

#endif

snakeGame.cpp

#include"snakeGame.h"
#include<SFML/Graphics.hpp>
#include<map>
#include<ctime>
#include<iostream>
#include<unistd.h>

void SnakeGame::mainLogic(){
    initWindow();
    while(window.isOpen()){
        updateEvents();
        updateMovements();
        draw();
    }
}

/*********************************************************************************************************
/
/
                                    / PRIVATE FUNCTIONS
/
/
/
/
**********************************************************************************************************/

void SnakeGame::initWindow(){
    srand(time(0));
    initializeEventsMap();
    window.create( sf::VideoMode(widthWindow, heightWindow), "SnakeGame", sf::Style::Titlebar);
    loadImage("images/red.png", texRed, redFoodSprite);
    loadImage("images/green.png", texGreen, greenSnakeSprite);
}

void SnakeGame::snakeMovements(){
    moveSnakeForward();
    if(snakeAteFood()){
        ++sizeOfSnake;
        delayTimeDecrease();
    }
    snakeOutOfFrame(); //change coordinates when snake is beyond of frame
    turnSnakeUp();
}

void SnakeGame::updateEvents(){
    while(window.pollEvent(event)){
        if(event.type == sf::Event::Closed)
            window.close();
    }
}

void SnakeGame::updateMovements(){
    snakeMovements();
    chooseMethodFromKeyboardEvents(codeFromKeyboard);
    codeFromKeyboard=event.key.code; // take direction form keyboard
    snakeSpeed(timeDelay);

}

void SnakeGame::chooseMethodFromKeyboardEvents(sf::Keyboard::Key codeFromKeyboard){
    auto iterator = mapOfEvents.find(codeFromKeyboard);

        if(iterator!=mapOfEvents.end()){
            iterator->second->event(*this);//left , right,up , down, pause
            mainEvent=codeFromKeyboard;
        }
        else mapOfEvents[mainEvent]->event(*this);
}

bool SnakeGame::snakeAteFood(){
    if(collisionSnakeWithFood()){
        makeNewFood();
        return true;
    }
    return false;
}

bool SnakeGame::collisionSnakeWithFood(){
    return (snake[0].xCoor==food.xCoor) && (snake[0].yCoor==food.yCoor);
}

bool SnakeGame::enmeshingSnake(){
    for(int i=2;i<sizeOfSnake;++i)
        if(selfCollision(i))
            return true;
    return false;
}

bool SnakeGame::selfCollision(int i){
    return ((snake[0].xCoor==snake[i].xCoor) && (snake[0].yCoor==snake[i].yCoor));
}

void SnakeGame::snakeOutOfFrame(){
    if(snake[0].yCoor>height-2){
        snake[0].yCoor=0;
    }
    if(snake[0].xCoor<0){
        snake[0].xCoor=width;
    }
    if(snake[0].xCoor>width){
        snake[0].xCoor=0;
    }
    if(snake[0].yCoor<0){
        snake[0].yCoor=height-2;
    }
}
void SnakeGame::snakeSpeed(int delay){
    usleep(delay);
}
void SnakeGame::delayTimeDecrease(){
    timeDelay=timeDelay-1500;
}

void SnakeGame::initializeEventsMap(){
    mapOfEvents[sf::Keyboard::Right] = std::shared_ptr<KeyboardEvents>(new GoRight);
    mapOfEvents[sf::Keyboard::Left] = std::shared_ptr<KeyboardEvents>(new GoLeft);
    mapOfEvents[sf::Keyboard::Up] = std::shared_ptr<KeyboardEvents>(new GoUp);
    mapOfEvents[sf::Keyboard::Down] = std::shared_ptr<KeyboardEvents>(new GoDown);
    mapOfEvents[sf::Keyboard::Space] = std::shared_ptr<KeyboardEvents>(new Pause);
    mapOfEvents[sf::Keyboard::Escape] = std::shared_ptr<KeyboardEvents>(new Exit);
}

void SnakeGame::draw(){
    window.clear();
    drawScreen(redFoodSprite, greenSnakeSprite, window);
    window.display();
}

void SnakeGame::drawScreen( sf::Sprite& redFood, sf::Sprite& greenSnake, sf::RenderWindow& window){

    for(int i=0;i<sizeOfSnake;++i){
            greenSnake.setPosition(snake[i].xCoor*size, snake[i].yCoor*size);
            window.draw(greenSnake);
        }
        redFood.setPosition(food.xCoor*size,food.yCoor*size);
        window.draw(redFood);
}

void SnakeGame::loadImage( const std::string& path, sf::Texture& tex, sf::Sprite& sprite){
    tex.loadFromFile(path);
    sprite.setTexture(tex);
}

void SnakeGame::moveSnakeForward(){
    for(int i=sizeOfSnake;i>0;--i){ // slither snake
        snake[i].xCoor=snake[i-1].xCoor;
        snake[i].yCoor=snake[i-1].yCoor;
    }
}

void SnakeGame::turnSnakeUp(){
    if(enmeshingSnake()){
        if(sizeOfSnake>5){
            sizeOfSnake=sizeOfSnake-3;
            timeDelay=timeDelay+3000; // come back to orginal timeDelay

        }
    }
}

void SnakeGame::makeNewFood(){
    food.xCoor = rand()%width;
    food.yCoor = rand()%height;
}
/*********************************************************************************************************
/
/
/                                    EVENTS FORM KEYBOARD FAMILLY 
/
/
/
/
**********************************************************************************************************/

/*********************************************************************************************************
 * DIRECTIONS
**********************************************************************************************************/
void SnakeGame::GoRight::event(SnakeGame& snakeObj){
    snakeObj.snake[0].xCoor+=1;
}
void SnakeGame::GoLeft::event(SnakeGame& snakeObj){
    snakeObj.snake[0].xCoor-=1;
}
void SnakeGame::GoUp::event(SnakeGame& snakeObj){
    snakeObj.snake[0].yCoor-=1;
}
void SnakeGame::GoDown::event(SnakeGame& snakeObj){
    snakeObj.snake[0].yCoor+=1;
}


/*********************************************************************************************************
 * SPECIAL EVENTS 
**********************************************************************************************************/

void SnakeGame::Pause::event(SnakeGame& snakeObj){ }

void SnakeGame::Exit::event(SnakeGame& snakeObj){
    snakeObj.window.close();
}
\$\endgroup\$
4
  • 5
    \$\begingroup\$ 1) I would separate this class into classes. I.e. a snake class for controlling the snake and a window class for managing the window. 2) Your function names should be actions, not really nouns. \$\endgroup\$ Sep 27, 2016 at 17:22
  • \$\begingroup\$ @ArchieGersman please add an answer instead of a comment. Refer to the section When shouldn't I comment? on Comment everywhere. \$\endgroup\$ Nov 21, 2017 at 17:11
  • \$\begingroup\$ @Klemens - the link provided doesn't appear to work... was the repository removed? \$\endgroup\$ Nov 21, 2017 at 17:12
  • 1
    \$\begingroup\$ Spelling: length, not lenght! \$\endgroup\$ Jan 29, 2019 at 9:19

1 Answer 1

4
\$\begingroup\$

Proper #include Guards

#ifndef _SNAKEGAME_H
#define _SNAKEGAME_H

There are two problems with this

  1. 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.)
  2. 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<sf::Keyboard::Key, std::shared_ptr<KeyboardEvents>> 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 <random> 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 <cstdlib>.


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.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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