Improved snake game in sfml (C++) #2

Question

So i took even more suggestions and improved the code from this question. Some of the suggestions i wasn't able to implement simply because of my "newbieness" in c++ programming

main.cpp

#include "app.h"

int main() {

    // Window aspect ratio is always 4:3, so it only takes
    // a value that is then used as the screen width.
    // The screen height is calculated in the Game::app class constructor.
    Game::app game(800, L"Test");
    game.start();
}

app.h

#pragma once

#include <SFML/Graphics.hpp>

#include "Board.h"

namespace Game {
    class app : public sf::Drawable {
    public:

        app(int windowWidth, const wchar_t* name);
        ~app() = default;       

        // Runs the app
        void start();
        void end();

    private:

        // MEMBER VARIABLES

        const float common_divisor;
        bool m_iscrashed;
        Board board;
        sf::RenderWindow window;
        sf::Font arial;

        // MEMBER FUNCTIONS

        void draw(sf::RenderTarget& target, sf::RenderStates states) const override;
        void handleEvents();
        void updateWindow();
    };
}

app.cpp

#include "app.h"

#include <iostream>
#include <thread>
#include <chrono>

Game::app::app(int windowWidth, const wchar_t* name)
    : common_divisor{ static_cast<float>(windowWidth / Board::width) }, m_iscrashed{ false } {

    const int windowHeight = windowWidth * 3 / 4;
    if (windowHeight % 3 != 0)
        std::wcout << L"Error! the window aspect ratio isn't 4:3.\n";

    const char* font_path{ "res/fonts/arial.ttf" };

    // Had to make it a normal string because
    // the loadFromFile() func doesn't accept wide strings.
    if (!arial.loadFromFile(font_path)) {
        std::cout << "[ERROR]: Couldn't load font\nFile Path: " << font_path << '\n\n';
    }

    window.create(sf::VideoMode(windowWidth, windowHeight), name);
    window.setFramerateLimit(5);
}

void Game::app::handleEvents() {

    sf::Event event;

    while (window.pollEvent(event)) {
        switch (event.type) {

        case sf::Event::Closed:
            window.close();
            break;

        case sf::Event::TextEntered:
            board.changeDirection(static_cast<char>(event.text.unicode));
        }
    }
}

void Game::app::draw(sf::RenderTarget& target, sf::RenderStates states) const {

    for (size_t i = 0, h = Board::height; i < h; ++i) {
        for (size_t j = 0, w = Board::width; j < w; ++j) {

            Coord here{ j, i };
            sf::RectangleShape rect;
            rect.setSize({ common_divisor, common_divisor });
            rect.setPosition({ common_divisor * j, common_divisor * i });

            switch (board.at(here)) {
            case Board::WALL:
                target.draw(rect, states);
                break;

            case Board::SNAKE:
                rect.setFillColor(sf::Color::Green);
                target.draw(rect, states);
                break;

            case Board::FOOD:
                rect.setFillColor(sf::Color::Red);
                target.draw(rect, states);

            }

        }
    }

    // Draws the game score
    sf::Text text;
    text.setFont(arial);
    text.setCharacterSize(static_cast<unsigned int>(common_divisor));
    text.setPosition({ 0.0f, 0.0f });
    text.setString("Score: " + std::to_string(board.score()));
    text.setFillColor(sf::Color::Black);

    target.draw(text, states);
}

// Updates the render window
void Game::app::updateWindow() {
    if (m_iscrashed)
        window.close();

    window.clear(sf::Color::Black);
    window.draw(*this);
    window.display();
}

// Starts the app
void Game::app::start() {

    while (window.isOpen()) {
        handleEvents();
        board.update(&m_iscrashed);
        updateWindow();
    }

    end();
}

void Game::app::end() {

    std::wcout << L"Game over!\nScore: " << board.score() << L'\n';
    std::this_thread::sleep_for((std::chrono::milliseconds)3000);   
}

Board.h

#pragma once    

#include "Snake.h"  

class Board {
public:

    Board();
    ~Board() = default;

    void update(bool* iscrashed);
    void changeDirection(char input);

    char operator[](int i)    const;
    int score()               const;
    int at(Coord coord)       const;

    static constexpr int width = 20;
    static constexpr int height = 15;

    static enum Tile {
        OPEN = 1,
        WALL = 2,
        SNAKE = 3,
        FOOD = 4
    };

private:


    // MEMBER VARIABLES
    Snake snake;
    std::string map;
    int m_score = 0;

    // MEMBER FUNCTIONS

    Coord openRandom();                 // Finds a random open cell
    bool place(Coord coord, Tile item); // Sets a cell a certain value  
    bool isEmpty(Coord coord) const;

};

Board.cpp

#include "Board.h"

#include <random>

Board::Board()
    : map(static_cast<size_t>(width * height), static_cast<char>(OPEN)) {

    // Sets top and bottom walls
    for (size_t i = 0; i < width; ++i) {
        place({ i, 0 }, WALL);
        place({ i, height - 1 }, WALL);
    }

    // Sets side walls
    for (size_t j = 1; j < height - 1; ++j) {
        place({ 0, j }, WALL);
        place({ width - 1, j }, WALL);
    }

    place(snake.headLocation(), SNAKE);
    place(snake.add(), SNAKE);

    place(openRandom(), FOOD);
}

int Board::at(Coord coord) const {
    return map[coord.y * width + coord.x];
}

bool Board::isEmpty(Coord coord) const {
    return at(coord) == OPEN;   
}

// Sets a cell a certain value
bool Board::place(Coord coord, Tile item) {
    if (item != OPEN && !isEmpty(coord))
        return false;

    map[coord.y * width + coord.x] = item;
    return true;
}

Coord Board::openRandom() {

    std::random_device rd;
    std::mt19937 gen(rd());

    std::uniform_int_distribution<unsigned int> disX(1, width - 2);
    std::uniform_int_distribution<unsigned int> disY(1, height - 2);

    Coord coord = { disX(gen), disY(gen) };

    while (!isEmpty(coord)) {
        coord = { disX(gen), disY(gen) };
    }

    return coord;
}

void Board::update(bool* iscrashed) {
    auto newHead{ snake.moveHead() };
    place(snake.follow(), OPEN);

    switch (at(snake.headLocation())) {
    case WALL:
    case SNAKE:
        *iscrashed = true;
        break;

    case FOOD:
        place(snake.headLocation(), OPEN);
        place(snake.add(), SNAKE);
        m_score += 100;

        place(openRandom(), FOOD);
    }

    place(newHead, SNAKE);  
}

void Board::changeDirection(char input) {
    snake.changeDirection(input);
}

char Board::operator[](int i) const { return map[i];  }
int Board::score()            const { return m_score; }

Snake.h

#pragma once

#include <vector>

#include "Coord.h"

class Snake {
public:

    Snake();
    ~Snake() = default;

    // Changes the dir value based on the input
    void changeDirection(char input);

    // Adds a piece to the snake and returns its location
    Coord add();
    size_t size();  

    /* Moves all pieces and returns
    the previous position of last piece */
    Coord follow();
    Coord moveHead(); // Moves and returns position of new head
    Coord headLocation() const;

private:    

    // MEMBER VARIABLES 
    struct Snake_segment
    {
        Coord current, previous;
    };

    enum direction {
        UP = 0,
        RIGHT,
        DOWN,
        LEFT
    };

    std::vector<Snake_segment> snakeContainer;
    direction dir;
};

Snake.cpp

#include "Snake.h"

// Initializes a two-piece snake
Snake::Snake()
    : dir { RIGHT } {

    Snake_segment head{ {10, 7}, {9, 7} };
    snakeContainer.push_back(head);

    --head.current.x;
    snakeContainer.push_back(head);
}

Coord Snake::add() {
    snakeContainer.push_back({
        snakeContainer.back().previous,
        snakeContainer.back().previous
    });

    return snakeContainer.back().current;
}

size_t Snake::size() {
    return snakeContainer.size();
}

// Changes the direction based on input (BUGGED)
void Snake::changeDirection(char input) {
    switch (input) {
    case 'w':
        if (dir != DOWN) dir = UP;
        break;

    case 'd':
        if (dir != LEFT) dir = RIGHT;
        break;

    case 's':
        if (dir != UP) dir = DOWN;
        break;

    case 'a':
        if (dir != RIGHT) dir = LEFT;

    }
}

// All the pieces follow the head
Coord Snake::follow() {

    auto it = snakeContainer.begin();
    for (auto prev = it++; it != snakeContainer.end(); ++it, ++prev) {
        it->previous = it->current;
        it->current = prev->previous;
    }

    return snakeContainer.back().previous;
}


Coord Snake::moveHead() {

    snakeContainer[0].previous = snakeContainer[0].current;

    switch (dir) {
    case UP:
        --snakeContainer.front().current.y;
        break;

    case RIGHT:
        ++snakeContainer.front().current.x;
        break;

    case DOWN:
        ++snakeContainer.front().current.y;
        break;

    case LEFT:
        --snakeContainer.front().current.x;

    }

    return snakeContainer.front().current;
}

Coord Snake::headLocation()                   const { return snakeContainer.front().current; }

Coord.h

#pragma once

struct Coord {
    unsigned int x, y;
};

Answer 1

Don't Ignore Warning Messages
Please compile using the -Wall compiler switch that provides all warning messages.

This code in Board.h generates a warning message:

static enum Tile {
    OPEN = 1,
    WALL = 2,
    SNAKE = 3,
    FOOD = 4
};

The keyword static is ignored in this case, why are you trying to make an enum static?

Style and How it Effects Maintainability
Changing the coding style can improve readability and maintainability of the code.

Suggested Style Change 1
Put open braces ({) on a new line. This improves readability by clearly showing where a code block starts and ends.

Example 1: Currently the function app::draw() looks like this:

void Game::app::draw(sf::RenderTarget& target, sf::RenderStates states) const {

    for (size_t i = 0, h = Board::height; i < h; ++i) {
        for (size_t j = 0, w = Board::width; j < w; ++j) {

            Coord here{ j, i };
            sf::RectangleShape rect;
            rect.setSize({ common_divisor, common_divisor });
            rect.setPosition({ common_divisor * j, common_divisor * i });

            switch (board.at(here)) {
            case Board::WALL:
                target.draw(rect, states);
                break;

            case Board::SNAKE:
                rect.setFillColor(sf::Color::Green);
                target.draw(rect, states);
                break;

            case Board::FOOD:
                rect.setFillColor(sf::Color::Red);
                target.draw(rect, states);

            }

        }
    }

    // Draws the game score
    sf::Text text;
    text.setFont(arial);
    text.setCharacterSize(static_cast<unsigned int>(common_divisor));
    text.setPosition({ 0.0f, 0.0f });
    text.setString("Score: " + std::to_string(board.score()));
    text.setFillColor(sf::Color::Black);

    target.draw(text, states);
}

The code might be more readable for others if the code looked like this:

void Game::app::draw(sf::RenderTarget& target, sf::RenderStates states) const
{
    for (size_t i = 0; i < Board::height; ++i)
    {
        for (size_t j = 0; j < Board::width; ++j)
        {
            Coord here{ j, i };
            sf::RectangleShape rect;
            rect.setSize({ common_divisor, common_divisor });
            rect.setPosition({ common_divisor * j, common_divisor * i });

            switch (board.at(here)) {
            case Board::WALL:
                target.draw(rect, states);
                break;

            case Board::SNAKE:
                rect.setFillColor(sf::Color::Green);
                target.draw(rect, states);
                break;

            case Board::FOOD:
                rect.setFillColor(sf::Color::Red);
                target.draw(rect, states);
            }    
        }
    }

    // Draws the game score
    sf::Text text;
    text.setFont(arial);
    text.setCharacterSize(static_cast<unsigned int>(common_divisor));
    text.setPosition({ 0.0f, 0.0f });
    text.setString("Score: " + std::to_string(board.score()));
    text.setFillColor(sf::Color::Black);

    target.draw(text, states);
}

Due to the fact that the code leaves a blank line after each open brace this doesn't change the vertical spacing very much, anyone that reads the code can find the matching braces and knows the extent of each code block.

Please Note that in the suggested version h = Board:height and w = Board:width are removed from the nest for loops, to improve readability as well, they are not necessary and they are confusing. It is better to avoid single character variable names. It might also be better to rename i and j as x(Point) and y(Point) or h(eight) and w(idth) to show that they are coordinates.

Suggested Style Change 2
Put variable declarations on separate lines, it is easier to find variables and to add or delete them when a variable declared on a separate line.

struct Coord {
    unsigned int x, y;
};

versus

struct Coord {
    unsigned int x;
    unsigned int y;
};

and

    struct Snake_segment
    {
        Coord current, previous;
    };

versus 

    struct Snake_segment
    {
        Coord current;
        Coord previous;
    };

Suggested Style Change 3
Put braces ({ and }) around all then clauses and else clauses. Quite often maintenance requires additional statements in either the then or else clauses. If the braces are not there then it is very easy to insert bugs into code by adding a statement.

void Game::app::updateWindow() {
    if (m_iscrashed)
        window.close();

    window.clear(sf::Color::Black);
    window.draw(*this);
    window.display();
}

Versus

void Game::app::updateWindow()
{
    if (m_iscrashed)
    {
        window.close();
    }

    window.clear(sf::Color::Black);
    window.draw(*this);
    window.display();
}