6
\$\begingroup\$

I reviewed this question to a snake game console implementation: My first C++ game (snake console game)

I enjoyed refactoring this code and presenting a solution using more C++ features/classes. I ended up rewriting this project from scratch.

My aim was to make the code easy and maintainable to read. Also, I tried to seperate the IO with the console from the logic because maybe I want to use the logic to port the game from console to QT-GUI as another excercise later.

I wonder what can be still be improved in the code?

Is the code easy to read for you/easy to follow?

Are there any bad practices?

Things which can possible be improved:

  • Currently, we are not portable. ConsoleOperations.cpp uses Windows specified header. Is there an easy way to enable Linux/Mac aswell?

main.cpp

#include "Game.h"

#include <iostream>

int main() 
try {
    snakeGame::runGame();
    return 0;
}
catch (...) {
    std::wcerr << "unknown error " << "\n";
    std::wcin.get();
}

Game.h

#pragma once

namespace snakeGame {

    void runGame();
}

namespace snakeGame::impl {

    class Board; // fwd delaration

    bool askUserToEndGame();

    void pauseUntilPauseKeyPressedAgain();

    void printBoardWithStats(const Board& board, long long score, int delay);

    void waitFor(int milliseconds);

    void printGameOverWithScore(int score);

}

Game.cpp

#include "Game.h"

#include "Snake.h"
#include "Board.h"
#include "ConsoleOperations.h"
#include "SnakeDirection.h"

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

namespace snakeGame {

    using namespace impl;

    void runGame()
    {
        for (;;) {

            if (askUserToEndGame()) {
                return;
            }

            constexpr auto fieldWidth = 40;
            constexpr auto fieldHeight = 15;

            Board board{ fieldWidth, fieldHeight };
            board.updateSnakePosition();
            board.placeFood();
            SnakeDirection snakeDirection = SnakeDirection::right;

            long long score{ 0 };
            long long points{ 100 };
            auto delay(300);

            bool wasPausedInLastLoop{ false };
            for (;;) {
                putCursorToStartOfConsole();
                printBoardWithStats(board, score, delay);

                if (wasPausedInLastLoop) {
                    // If we don't do this and print pause to the console by 
                    // pressing p during the game the pause statement will 
                    // still be printed because  during the game the pause 
                    // statement will still be printed because during the game 
                    // the pause statement will still be printed because 
                    // during the game the pause statement will still be 
                    // printed because we start printing from the beginning of
                    // the console and now the total string printed to the 
                    // console would be one row lower.
                    std::wcout << L"                                     \n";
                    wasPausedInLastLoop = false;
                }

                if (keyWasPressed()) {
                    auto key = getKey();

                    if (key == 'p') {
                        wasPausedInLastLoop = true;
                        std::wcout << L"#####PAUSED#####\n";
                        pauseUntilPauseKeyPressedAgain();
                    }
                    else {
                        snakeDirection = updateDirection(key, snakeDirection);
                    }
                }

                board.moveSnake(snakeDirection);

                if (board.snakeHitFood()) {
                    board.eatFood();
                    board.growSnake();
                    board.placeFood();
                    score += points;
                    points *= 2;
                    delay -= 5;
                }
                else if (board.snakeHitWall() || board.snakeHitSnake()) {
                    break;
                }
                board.updateSnakePosition();

                std::this_thread::sleep_for(std::chrono::milliseconds{ delay });
            }

            printGameOverWithScore(score);
        }
    }
}

namespace snakeGame::impl {

    bool askUserToEndGame()
    {
        clearScreen();
        while (true) {

            auto choice{ 0 };
            std::wcout << L"1. Play\n";
            std::wcout << L"2. Quit\n";
            std::wcin >> choice;

            if (choice == 1) {
                return false;
            }

            else if (choice == 2) {
                return true;
            }

            else {
                std::wcout << L"Invalid input!";
                std::wcin.get();
                clearScreen();
            }
        }
    }

    void pauseUntilPauseKeyPressedAgain()
    {
        for (;;) {
            if (keyWasPressed) {

                auto key = getKey();

                if (key == 'p') {
                    return;
                }
            }
        }
    }

    void printBoardWithStats(const Board& board, long long score, int delay)
    {
        std::wcout << L"Score:" << score << '\n';
        std::wcout << L"Delay:" << delay << "ms  \n";
        std::wcout << board;
        std::wcout << L"Use 'w, a, s, d' to change directions\n";
    }

    void waitFor(int milliseconds)
    {
        std::this_thread::sleep_for(std::chrono::milliseconds{ milliseconds });
    }

    void printGameOverWithScore(int score)
    {
        clearScreen();
        std::wcout << L"Game over!\n";
        std::wcout << L"Score: " << score << '\n';
        std::wcin.clear();
        std::wcin.ignore(120, '\n');
        std::wcin.get();
    }
}

Board.h

#pragma once

#include "Snake.h"

#include <vector>
#include <random>
#include <iosfwd>

namespace snakeGame::impl {

    enum class SnakeDirection;

    struct Element {
        bool hasSnakeSegment{ false };
        bool hasSnakeHead{ false };
        bool hasWall{ false };
        bool hasFood{ false };
    };

    class Board
    {
    public:
        Board(int width, int height);

        void placeFood();
        void updateSnakePosition();
        bool snakeHitFood() const;
        void eatFood();
        void growSnake();
        bool snakeHitWall() const;
        bool snakeHitSnake() const;
        void moveSnake(SnakeDirection snakeDirection);

        void debugPrintSnakeCoordinates();
    private:
        std::vector<std::vector<Element>> initFieldWithWalls(int width, int height);
        void removeOldSnakePosition(const std::vector<SnakeSegment>& body);
        void addNewSnakePosition(const std::vector<SnakeSegment>& body);

        Snake mSnake;
        std::vector<std::vector<Element>> mField;

        std::random_device mRandomDevice;
        std::default_random_engine mGenerator;
        std::uniform_int_distribution<int> mWidthDistribution;
        std::uniform_int_distribution<int> mHeightDistribution;

        friend std::wostream& operator<<(std::wostream& os, const Board& obj);
    };

    std::wostream& operator<<(std::wostream& os, const Board& obj);

}

Board.cpp

#include "Board.h"

#include "SnakeDirection.h"

#include <algorithm>
#include <iostream>;

namespace snakeGame::impl {

    Board::Board(int width, int height)
        : mSnake{ width, height },
        mField{ initFieldWithWalls(width, height) },
        mRandomDevice{},
        mGenerator{ mRandomDevice() },
        mWidthDistribution{ 1, width - 2 },
        mHeightDistribution{ 1, height - 2 }
    {
    }

    void Board::updateSnakePosition()
    {
        auto snakeBody = mSnake.getBody();

        removeOldSnakePosition(snakeBody);
        addNewSnakePosition(snakeBody);
    }

    bool Board::snakeHitFood() const
    {
        auto pos = mSnake.getBody()[0].pos;
        return mField[pos.y][pos.x].hasFood;
    }

    void Board::eatFood()
    {
        auto pos = mSnake.getBody()[0].pos;
        mField[pos.y][pos.x].hasFood = false;
    }

    void Board::growSnake()
    {
        mSnake.grow();
    }

    bool Board::snakeHitWall() const
    {
        auto pos = mSnake.getBody()[0].pos;
        return mField[pos.y][pos.x].hasWall;
    }

    bool Board::snakeHitSnake() const
    {
        auto pos = mSnake.getBody()[0].pos;
        return mField[pos.y][pos.x].hasSnakeSegment;
    }

    void Board::moveSnake(SnakeDirection snakeDirection)
    {
        switch (snakeDirection) {
        case SnakeDirection::right:
            mSnake.moveRight();
            break;

        case SnakeDirection::down:
            mSnake.moveDown();
            break;

        case SnakeDirection::left:
            mSnake.moveLeft();
            break;

        case SnakeDirection::up:
            mSnake.moveUp();
            break;
        }
    }

    void Board::debugPrintSnakeCoordinates()
    {
        auto body = mSnake.getBody();

        for (auto i = 0; i < body.size(); ++i) {

            auto pos = body[i].pos;

            std::wcout << "nr:" << i << "x:" << pos.x << "\t" << "y:" << pos.y << "\t";

            auto field = mField[pos.y][pos.x];

            if (field.hasSnakeHead) {
                std::wcout << L"Head\t";
            }
            else {
                std::wcout << L"    \t";
            }
            if (field.hasSnakeSegment) {
                std::wcout << L"Body\n";
            }
            else {
                std::wcout << L"    \n";
            }
        }
    }

    void Board::placeFood()
    {
        for (;;) {
            auto x = mWidthDistribution(mGenerator);
            auto y = mHeightDistribution(mGenerator);

            if (!mField[y][x].hasSnakeHead &&
                !mField[y][x].hasSnakeSegment) {
                mField[y][x].hasFood = true;
                break;
            }
        }
    }

    std::vector<std::vector<Element>> Board::initFieldWithWalls(int width, int height)
    {
        std::vector<Element> row(width, Element{});
        std::vector<std::vector<Element>> field(height, row);

        Element wall{ false, false, true, false };

        std::fill(field[0].begin(), field[0].end(), wall);
        std::fill(field[field.size() - 1].begin(), field[field.size() - 1].end(), wall);

        for (auto it_row = field.begin() + 1; it_row < field.end() - 1; ++it_row) {
            (*it_row)[0] = wall;
            (*it_row)[it_row->size() - 1] = wall;
        }
        return field;
    }

    void Board::removeOldSnakePosition(const std::vector<SnakeSegment>& body)
    {
        auto first{ true };
        for (const auto& snakeSegment : body) {
            auto prev = snakeSegment.prev;

            if (first) {
                mField[prev.y][prev.x].hasSnakeHead = false;
                first = false;
            }
            else {
                mField[prev.y][prev.x].hasSnakeSegment = false;
            }
        }
    }

    void Board::addNewSnakePosition(const std::vector<SnakeSegment>& body)
    {
        auto first{ true };
        for (const auto& snakeSegment : body) {
            auto pos = snakeSegment.pos;

            if (first) {
                mField[pos.y][pos.x].hasSnakeHead = true;
                first = false;
            }
            else {
                mField[pos.y][pos.x].hasSnakeSegment = true;
            }
        }
    }


    std::wostream& operator<<(std::wostream& os, const Board& obj)
    {
        for (const auto& row : obj.mField) {
            for (const auto& element : row) {
                if (element.hasSnakeSegment) {
                    os << L'o';
                }
                else if (element.hasSnakeHead) {
                    os << L'@';
                }
                else if (element.hasWall) {
                    os << L'#';
                }
                else if (element.hasFood) {
                    os << L'*';
                }
                else {
                    os << L' ';
                }
            }
            os << '\n';
        }
        return os;
    }

}

Snake.h

#pragma once

#include "Point.h"

#include <vector>

namespace snakeGame::impl {

    struct SnakeSegment
    {
        Point pos{ 0 , 0 };
        Point prev{ pos };
    };

    class Snake
    {
    public:
        Snake(int boardWidth, int boardHeight);

        std::vector<SnakeSegment> getBody() const;

        void moveRight();
        void moveDown();
        void moveLeft();
        void moveUp();
        void grow();

    private:
        void safeCurrentPosToLastOfFirstElement();
        void moveRemainingElements();

        std::vector<SnakeSegment> mBody;
    };

    std::vector<SnakeSegment> initSnake(int fieldWidth, int fieldHeight);

}

Snake.cpp

#include "Snake.h"

namespace snakeGame::impl {

    Snake::Snake(int fieldWidth, int fieldHeight)
        :mBody{ initSnake(fieldWidth, fieldHeight) }
    {
    }

    std::vector<SnakeSegment> Snake::getBody() const
    {
        return mBody;
    }


    void Snake::moveRight()
    {
        safeCurrentPosToLastOfFirstElement();
        ++mBody[0].pos.x;
        moveRemainingElements();
    }

    void Snake::moveDown()
    {
        safeCurrentPosToLastOfFirstElement();
        ++mBody[0].pos.y;
        moveRemainingElements();
    }

    void Snake::moveLeft()
    {
        safeCurrentPosToLastOfFirstElement();
        --mBody[0].pos.x;
        moveRemainingElements();
    }

    void Snake::moveUp()
    {
        safeCurrentPosToLastOfFirstElement();
        --mBody[0].pos.y;
        moveRemainingElements();
    }

    void Snake::grow()
    {
        SnakeSegment newTail;
        newTail.pos.x = mBody[mBody.size() - 1].prev.x;
        newTail.pos.y = mBody[mBody.size() - 1].prev.y;
        mBody.push_back(newTail);
    }

    void Snake::safeCurrentPosToLastOfFirstElement()
    {
        mBody[0].prev.x = mBody[0].pos.x;
        mBody[0].prev.y = mBody[0].pos.y;
    }

    void Snake::moveRemainingElements()
    {
        for (int i = 1; i < mBody.size(); ++i) {
            mBody[i].prev.x = mBody[i].pos.x;
            mBody[i].prev.y = mBody[i].pos.y;
            mBody[i].pos.x = mBody[i - 1].prev.x;
            mBody[i].pos.y = mBody[i - 1].prev.y;
        }
    }


    std::vector<SnakeSegment> initSnake(int boardWidth, int boardHeight)
    {
        auto x = boardWidth / 2;
        auto y = boardHeight / 2;
        std::vector<SnakeSegment> body{
            SnakeSegment{ x, y },
            SnakeSegment{ x - 1, y },
        };
        return body;
    }

}

Point.h

#pragma once

namespace snakeGame::impl {

    struct Point {
        int x;
        int y;
    };

}

SnakeDirection.h

#pragma once

namespace snakeGame::impl {

    enum class SnakeDirection {
        up, right, down, left
    };

}

ConsoleOperations.h

#pragma once

// Non portable. At the moment only windows works

namespace snakeGame::impl {

    enum class SnakeDirection;

    void putCursorToStartOfConsole();

    void clearScreen();

    bool keyWasPressed();

    char getKey();

    SnakeDirection updateDirection(char c, SnakeDirection direction);
}

ConsoleOperations.cpp

#include "ConsoleOperations.h"

#include "SnakeDirection.h"

#include <cstdlib>

//#ifdef _WIN32
#include <conio.h>
#include <Windows.h>
//#else 
//  //Assume POSIX
//#endif

namespace snakeGame::impl {

    void putCursorToStartOfConsole()
    {
        //#ifdef _WIN32
        HANDLE hOut;
        COORD Position;

        hOut = GetStdHandle(STD_OUTPUT_HANDLE);

        Position.X = 0;
        Position.Y = 0;
        SetConsoleCursorPosition(hOut, Position);
        //#else 
        //  //Assume POSIX
        //#endif
    }


    void clearScreen()
    {
        //#ifdef _WIN32
        std::system("cls");
        //#else
        //  // Assume POSIX
        //  std::system("clear");
        //#endif
    }

    bool keyWasPressed()
    {
        //#ifdef _WIN32
        return static_cast<bool>(_kbhit());
        //#else
            // Assume POSIX
        //#endif
    }

    char getKey()
    {
        //#ifdef _WIN32
        return _getch();
        //#else
            // Assume POSIX
        //#endif
    }

    SnakeDirection updateDirection(char c, SnakeDirection direction)
    {
        switch (c) {
        case 'a':
            if (direction != SnakeDirection::right) {
                direction = SnakeDirection::left;
            }
            break;

        case 'w':
            if (direction != SnakeDirection::down) {
                direction = SnakeDirection::up;
            }
            break;

        case 'd':
            if (direction != SnakeDirection::left) {
                direction = SnakeDirection::right;
            }
            break;

        case 's':
            if (direction != SnakeDirection::up) {
                direction = SnakeDirection::down;
            }
            break;
        }
        return direction;
    }

}
\$\endgroup\$
  • \$\begingroup\$ I assume snakedirection is a header file, but you might want to make that clearer. \$\endgroup\$ – pacmaninbw May 8 at 14:31
  • \$\begingroup\$ What version of Visual Studio are you using? \$\endgroup\$ – pacmaninbw May 8 at 15:05
  • \$\begingroup\$ Fixed. Its a header file. I used MSVC2017. \$\endgroup\$ – Sandro4912 May 8 at 16:12
4
\$\begingroup\$

Overall a good job, and much better than the version you answered.

Better Not to Ignore Warning Messages
Board.cpp generates 2 warning messages, the first is for a typo on the semicolon on this line:

#include <iostream>;

The second is for a type mismatch on this line

    for (auto i = 0; i < body.size(); ++i) {

Auto comes in very handy for some things, but it is best not to abuse it. C++ is not a scripting language and type checking in C++ is a good thing. Use auto to define iterators when looping through a container but use the proper type in other instances. It's generally a good idea for someone who has to maintain the code to know what type an objects is. Unlike C# and some other languages C++ does not have Reflection.

Inlcude Header Files Within Headers
The code might be more maintainable if header files such as Board.h and ConsoleOperations.h included header files for objects they consume such as SnakeDirection.h and Point.h. As it is now someone using Board.h in a new file will run into compiler issues if they haven't already included the proper files.

Check User Input
Neither the function getKey() nor updateDirection(key, snakeDirection) performs adequate error checking, if the user enters an illegal value, the behavior is unknown. It is always good to check user input.

\$\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.