4
\$\begingroup\$

I haven't done any programming in a while, and I'm getting back into it. I made this Snake clone to refresh me a bit, and I would like to get some feedback in the "elegance" and "well-done"-ness of the code.

main.cpp

#include "Game.h"

int main()
{
    Game::run();
    return 0;
}

Game.h

#ifndef __GAME_H__
#define __GAME_H__

#include <SFML/Graphics.hpp>

#include <random>
#include <utility>
#include <vector>

enum class Direction {UP, RIGHT, DOWN, LEFT};
enum class MoveEvent {EAT, COLLIDE};

class Game
{
public:

    static void run();

private:

    static sf::RenderWindow *                   window;
    static int                                  score;
    static int                                  level;
    static bool                                 alive;

    static std::pair<int, int>                  foodLocation;

    // Random Number Generation for X
    static const int                            rangex_from;
    static const int                            rangex_to;
    static std::random_device                   rand_devX;
    static std::mt19937                         generatorX;
    static std::uniform_int_distribution<int>   distrX;

    static const int                            rangey_from;
    static const int                            rangey_to;
    static std::random_device                   rand_devY;
    static std::mt19937                         generatorY;
    static std::uniform_int_distribution<int>   distrY;

    static std::vector<std::pair<int, int>>     snake_body;
    static Direction                            snake_direction;
    static Direction                            new_direction;

    static std::pair<int, int>                  getNewFoodLocation();
    static void                                 moveSnake();
    static void                                 draw();
    static bool                                 collides(std::pair<int, int>, std::vector<std::pair<int, int>>);

    static sf::Texture tile;
    static sf::Texture body;
    static sf::Texture food;

    static sf::Sprite tile_spr;
    static sf::Sprite body_spr;
    static sf::Sprite food_spr;
};

#endif

Game.cpp

#include "Game.h"

#include <iostream>

// Initialization of static variables

sf::RenderWindow * Game::window = new sf::RenderWindow(sf::VideoMode(640, 480), "Snake", sf::Style::Titlebar | sf::Style::Close);
int Game::score = 0;
int Game::level = 1;
bool Game::alive = true;

const int Game::rangex_from =    0;
const int Game::rangex_to =     39;
const int Game::rangey_from =    0;
const int Game::rangey_to =     29;

std::random_device      Game::rand_devX;
std::random_device      Game::rand_devY;

std::mt19937            Game::generatorX(Game::rand_devX());
std::mt19937            Game::generatorY(Game::rand_devY());

std::uniform_int_distribution<int>   Game::distrX(Game::rangex_from, Game::rangex_to);
std::uniform_int_distribution<int>   Game::distrY(Game::rangey_from, Game::rangey_to);

std::vector<std::pair<int, int>>                    Game::snake_body;
Direction                                           Game::snake_direction;
Direction                                           Game::new_direction;
std::pair<int, int>                                 Game::foodLocation = Game::getNewFoodLocation();

sf::Texture                                         Game::body;
sf::Texture                                         Game::tile;
sf::Texture                                         Game::food;
sf::Sprite                                          Game::body_spr;
sf::Sprite                                          Game::tile_spr;
sf::Sprite                                          Game::food_spr;


std::pair<int, int> Game::getNewFoodLocation()
{
    std::pair<int, int> temporalLocation;

    do
    {
        temporalLocation = std::pair<int, int>(Game::distrX(Game::generatorX), Game::distrY(Game::generatorY));
    } while (collides(temporalLocation, Game::snake_body));

    return temporalLocation;
}

bool Game::collides(std::pair<int, int> coordinate, std::vector<std::pair<int, int>> body)
{
    for (int i = 0; i < body.size(); i++)
        if (coordinate.first == body[i].first && coordinate.second == body[i].second)
            return true;

    return false;
}

void Game::moveSnake()
{
    std::pair<int, int> new_head_pos;

    // Calculates the new position the snakes head will take
    switch (new_direction)
    {
    case Direction::UP:
        if (snake_body[0].second - 1 >= rangey_from)
            new_head_pos.second = snake_body[0].second - 1;
        else
            new_head_pos.second = rangey_to;
        new_head_pos.first = snake_body[0].first;       
        break;

    case Direction::DOWN:
        if (snake_body[0].second + 1 <= rangey_to)
            new_head_pos.second = snake_body[0].second + 1;
        else
            new_head_pos.second = rangey_from;
        new_head_pos.first = snake_body[0].first;       
        break;

    case Direction::LEFT:
        if (snake_body[0].first - 1 >= rangex_from)
            new_head_pos.first = snake_body[0].first - 1;
        else
            new_head_pos.first = rangex_to;
        new_head_pos.second = snake_body[0].second;
        break;

    case Direction::RIGHT:
        if (snake_body[0].first + 1 <= rangex_to)
            new_head_pos.first = snake_body[0].first + 1;
        else
            new_head_pos.first = rangex_from;
        new_head_pos.second = snake_body[0].second;
        break;
    }

    //Fast hack <_<
    std::vector<std::pair<int, int>> body_minus_tail = snake_body;
    body_minus_tail.pop_back();

    // Checks if the snake will collide with its own body
    if(collides(new_head_pos, body_minus_tail))
    { 
        std::cout << "You Lost!     Score: " << score << std::endl;
        alive = false;
    }

    if(new_head_pos == foodLocation)
    { 
        score++;
        snake_body.insert(snake_body.begin(), new_head_pos);
        foodLocation = getNewFoodLocation();
    }
    else
    {
        snake_body.insert(snake_body.begin(), new_head_pos);
        snake_body.pop_back();
    }
    snake_direction = new_direction;
}

void Game::run()
{
    window->setFramerateLimit(60);

    snake_body.push_back(std::pair<int, int>(2, 0));
    snake_body.push_back(std::pair<int, int>(1, 0));
    snake_body.push_back(std::pair<int, int>(0, 0));

    snake_direction = Direction::RIGHT;
    new_direction   = Direction::RIGHT;

    body.loadFromFile("body.png");
    tile.loadFromFile("tile.png");
    food.loadFromFile("food.png");

    body_spr.setTexture(body);
    tile_spr.setTexture(tile);
    food_spr.setTexture(food);

    sf::Clock clock;
    sf::Time time1;

    while (window->isOpen())

    {
        sf::Event event;

        while (window->pollEvent(event))
        {
            switch (event.type)
            {
                // Calculates the new direction the snake is heading based on user input
            case sf::Event::KeyPressed:
                switch (event.key.code)
                {
                case sf::Keyboard::Up:
                case sf::Keyboard::W:
                    if (snake_direction != Direction::DOWN)
                        new_direction = Direction::UP;
                    break;
                case sf::Keyboard::Right:
                case sf::Keyboard::D:
                    if (snake_direction != Direction::LEFT)
                        new_direction = Direction::RIGHT;
                    break;
                case sf::Keyboard::Down:
                case sf::Keyboard::S:
                    if (snake_direction != Direction::UP)
                        new_direction = Direction::DOWN;
                    break;
                case sf::Keyboard::Left:
                case sf::Keyboard::A:
                    if (snake_direction != Direction::RIGHT)
                        new_direction = Direction::LEFT;
                    break;
                }
                break;

            case sf::Event::Closed:
                window->close();
                break;
            }

        }

        time1 = clock.getElapsedTime();
        if (time1.asMilliseconds() >= 1000/20)
        {
            moveSnake();
            clock.restart();
        }

        if (!alive)
            window->close();

        window->clear();
        draw();
        window->display();

    }
}

void::Game::draw()
{
    for (int i = 0; i < rangex_to; i++)
        for (int j = 0; j < rangey_to; j++)
        {
            tile_spr.setPosition(i*16, j*16);
            window->draw(tile_spr);
        }

    food_spr.setPosition(foodLocation.first*16, foodLocation.second*16);
    window->draw(food_spr);

    for (int i = 0; i < snake_body.size(); i++)
    {
        body_spr.setPosition(snake_body[i].first*16, snake_body[i].second*16);
        window->draw(body_spr);
    }
}
\$\endgroup\$
5
\$\begingroup\$

Basic structure

With all its members static, your Game class looks a whole lot like a namespace. I'd either make it a namespace, or else make everything that's specific to a particular game (e.g., the current score) non-static so that each game object really represents an actual game. The latter is probably preferable, but either is an improvement on the current situation.

Worse, your Game class looks a lot like what's often called a "God class". A single class that tries to be all, know all, and do all. By lumping everything together into a single class, you've lost most of the benefits of using classes in the first place, and just written basic block-structured code about like you would in something like C, but with Game:: tacked onto the beginnings of many (most?) of the names.

I'd rather see (for example) one class for the snake, another for a food object, and (possibly) another for the game board (or possibly just for a tile).

Random number generation

I don't see any use in having separate random number generators for your X and Y coordinates. Quite the contrary, you're almost certainly better off using a single generator for both. Likewise, there's no point in creating separate instances of random_device to initialize each.

Wrapping numbers

A fair amount of the logic in your game code is devoted to the fairly simple job of ensuring that a number always stays within a particular range (and if you try to decrement below the beginning or increment above the end, wrapping around to the other end).

At least in my opinion, that sounds a lot like the specification of a small class:

class bounded { 
    int current;
    int lower;
    int upper;
public:
    bounded(int lower, int upper) : upper(upper), lower(lower) {}

    bounded &operator++() { 
       ++current;
       if (current >= upper)
           current = lower;
       return *this;
    }

    bounded &operator--() { 
        --current;
        if (current < lower)
            current = upper;
        return *this;
    }
};

For other purposes, you could add things like assignment, +=, -=, and so on, but for this game I believe increment and decrement are all you really need.

Using this, the code for the main loop of the game is simplified considerably:

case Direction::UP:
    --new_head_pos.second;
    new_head_pos.first = snake_body[0].first;       
    break;

// ...

Coordinates

Rather than use an std::pair<int, int> as the coordinate (and have all the other code know about its internals) I'd define a Coordinate class that knows how to do the things you have to do with coordinates (e.g., compare them):

class Coordinate {
    int x;
    int y;
public:
    Coordinate(int x, int y) : x(x), y(y) {}

    bool operator==(Coordinate const &b) const { 
        return x == b.x && y == b.y;
    }
};

Use of standard algorithms

Once you've defined Coordinate, your Game::collides becomes something like:

bool Game::collides(std::pair<int, int> coordinate, std::vector<std::pair<int, int>> body)
{
    for (int i = 0; i < body.size(); i++)
        if (coordinate == body[i])
            return true;
    return false;
}

This, in turn, can be written something like:

bool Game::collides(Coordinate const &coordinate, std::vector<Coordinate> const &body) { 
     return std::find(body.begin(), body.end(), coordinate) != body.end();
}

Of course, if you take the previous advice, the external interface to this would simplified a bit further, because it would be a member of the Snake class:

class Snake { 
    std::vector<Coordinate> body;
public:
    bool collides(Coordinate const &point) { 
        return std::find(body.begin(), body.end(), point) != body.end();
    }

    // ...        
};
\$\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.