Snake Game in C++ using OOP

Question

I am currently a student and I have an assignment in my OOP C++ course where I need to create a snake game using these specific requirements. I added some features into snake to fit these requirements

1. Must use classes, objects, and functions
2. Must use inheritance or polymorphism
3. Must use vectors, arrays and or structs
4. enumerators or lists

Any feed back would be greatly appriciated! My main goal is making it more coherent and efficient / shortening code!

#include <iostream>
#include <conio.h>
#include <windows.h>
#include <iomanip> 
#include <string>
#include <vector>
#include <stdlib.h>
#include <time.h>
#define KEY_UP 72
#define KEY_DOWN 80
#define KEY_LEFT 75
#define KEY_RIGHT 77
using namespace std;

class gameInfo {
public:
    vector<int> score = { 60, 40, 20 };
    vector<int> snakeBodyX;
    vector<int> snakeBodyY;

    bool isAlive = true, playerWon = false;

    int playFieldWidth = 30;
    int playFieldHeight = 20;
    int x = playFieldWidth / 2, y = playFieldHeight / 2;
    int foodPosX = rand() % (playFieldWidth - 4) + 2;
    int foodPosY = rand() % (playFieldHeight - 4) + 2;
    int min = 0, row, col, currentScore = 0, currentBest = 0, gamesPlayed = 0;
    int tillHighScore = score[2];

    void updateCurrentBest() { 
        if (currentScore > currentBest) currentBest = currentScore; 
    }

    void updateTillHighScore() {  
        for (int i = 0; i < 3; i++) {
            if (currentScore < score[i]) tillHighScore = score[i] - currentScore;
            else if (currentScore >= score[0]) tillHighScore = 0;
        } 
    } 

    void updateScoreBoard() {  
        for (int tracker = 0; tracker < score.size(); tracker++) {
            if (currentScore > score[tracker]) { 
                score.insert(score.begin() + tracker, currentScore);
                score.erase(score.begin()+score.size());
                return;
            }
        }
    }
};

class snakeInteractions : public gameInfo {
public:
    void getSnakeBody() {
        snakeBodyX.insert(snakeBodyX.begin(), x);
        snakeBodyY.insert(snakeBodyY.begin(), y);
    }

    void resetSnakeBody() {
        for (int erase = 0; erase < snakeBodyX.size(); erase++) {
            snakeBodyX.erase(snakeBodyX.begin() + erase);
            snakeBodyY.erase(snakeBodyY.begin() + erase);
        }
    }

    void updateFoodPOS() { // randomly places food between 2 and (dimension - 2) - aka doesn't touch walls because I suck at this game and keep dying
        bool isUnique = false;
        while (!isUnique) {
            foodPosX = rand() % (playFieldWidth - 4) + 2;
            foodPosY = rand() % (playFieldHeight - 4) + 2;

            for (int i = 0; i < snakeBodyX.size(); i++) {
                if (foodPosY == snakeBodyY[i] && foodPosX == snakeBodyX[i]) break;
                else isUnique = true;
            }
        }
    }

    void getSnakeCondition() { // just checks for if the snake has eatten a piece of food or died by crashing into itself or a wall.
        if (foodPosX == x && foodPosY == y) {
            currentScore += 10;
            updateTillHighScore();
            updateFoodPOS();
        }

        if (x == 0 || x == playFieldWidth - 1 || y == 0 || y == playFieldHeight - 1) { 
            isAlive = false; 
        }

        for (int i = 1; i <= currentScore / 10; i++) {
            if (snakeBodyX[i] == x && snakeBodyY[i] == y) {
                isAlive = false;
            }
        }
    }
};



class playSnake : public snakeInteractions { 
public:

    enum directions { UP, DOWN, LEFT, RIGHT };
    directions dir;

    void getKeyStroke() {
        if (_kbhit()) {
            switch (_getch()) {
            case 'a': case 'A': case KEY_LEFT:
                if (dir != RIGHT) dir = LEFT;
                break;
            case 'd': case 'D': case KEY_RIGHT:
                if (dir != LEFT) dir = RIGHT;
                break;
            case 'w': case 'W': case KEY_UP:
                if (dir != DOWN) dir = UP;
                break;
            case 's': case 'S': case KEY_DOWN:
                if (dir != UP) dir = DOWN;
                break;
            }
        }
    }

    void setPlayField() {
        system("cls");
        for (row = 0; row < playFieldHeight; row++) {
            for (col = 0; col < playFieldWidth; col++) {
                if (row == 0 || row == playFieldHeight - 1) cout << "*";
                else if (col == 0 || col == playFieldWidth - 1) cout << "*";
                else if (row == y && col == x) cout << "X";
                else if (row == foodPosY && col == foodPosX) cout << "O";
                else {
                    bool showSpace = true; // is there a better way of doing this? This feels sloppy
                    for (int body = 1; body < (currentScore+10)/10; body++) {
                        if (snakeBodyX[body] == col && snakeBodyY[body] == row) {
                            cout << "X";
                            showSpace = false;
                        }
                    }
                    if (showSpace) {
                        cout << " ";
                    }
                }
            }
            setScoreBoard(row);
            cout << endl;
        }
    }

    void setScoreBoard(int row) { // is there a more efficient way of doing this? I couldn't think of one.
        if (row == 1) cout << setw(22) << "Current Score:" << setw(13) << currentScore;
        if (row == 2) cout << setw(26) << "Until High Score: " << setw(9) << tillHighScore; 
        if (row == 5) cout << setw(35) << "Top Scores                 ";
        if (row == 6) cout << setw(35) << "===========================";
        if (row == 7) cout << setw(17) << "1st Place" << setw(18) << score[0];
        if (row == 8) cout << setw(17) << "2nd Place" << setw(18) << score[1];
        if (row == 9) cout << setw(17) << "3rd Place" << setw(18) << score[2];
        if (row == 12) cout << setw(35) << "Current Session            ";
        if (row == 13) cout << setw(35) << "===========================";
        if (row == 14) cout << setw(21) << "Games Played:" << setw(14) << gamesPlayed;
        if (row == 15) cout << setw(21) << "Current Best:" << setw(14) << currentBest; 
    }

    void getSnakeMovement() { 
        if (dir == LEFT) x--;
        else if (dir == RIGHT) x++;
        else if (dir == UP) y--;
        else if (dir == DOWN) y++;
        else return;
    }

    void playAgain() { // is there a better way of doing this? 
        x = playFieldWidth / 2;
        y = playFieldHeight / 2;
        gamesPlayed++;
        resetSnakeBody();
        updateFoodPOS();
        updateCurrentBest();
        updateScoreBoard();
        currentScore = 0;
        isAlive = true;
        playGame();
    }


    void playGame() { 
        while (isAlive) {
            setPlayField();
            getKeyStroke();
            getSnakeMovement();
            getSnakeBody();
            getSnakeCondition();
            if (!isAlive) playAgain(); // currently not a boolean, just takes true for testing reasons
            Sleep(100);         
        }
    }
};

int main() { // was always taught it was a good practice from my professor to keep main as minimal and clean as possible.
    srand(time(NULL));

    playSnake play;
    play.playGame();

    system("PAUSE");
}

Answer 1

Was this ported from an old C program, to incorporate the C++ features asked? I wonder because #define should not even be taught for making constants like that in a C++ class.

---

Read through and bookmark the C++ Standard Guidelines. Numbers I note later are citations from this.

---

Don’t write using namespace std;.

You can, however, put individual using std::string; etc. in a CPP file (not H file) or inside a function (See SF.7.)

---

Don't use #define for constants or "functions" (⧺ES.31).

---

int min = 0, row, col, currentScore = 0, currentBest = 0, gamesPlayed = 0;

Don’t define more than one member in one declaration.

---

void updateCurrentBest() { 
    if (currentScore > currentBest) currentBest = currentScore; 
}

First of all, why is that public? I can’t imagine why the client of the class would need to manually trigger the update. The highest scores should simply be remembered, through the game scoring/ending process.

Write code that shows intent, using named functions and library functions. Here, it is clearer and more efficient to write:

currentBest = std::max(currentBest,currentScore);

---

    for (int i = 0; i < 3; i++) {
        if (currentScore < score[i]) tillHighScore = score[i] - currentScore;
        else if (currentScore >= score[0]) tillHighScore = 0;
    } 

Don’t loop through indexes if you can avoid it. You want to handle each element in the score collection, right? So say that!

for (const auto& item : score) {
    if (currentScore < item) ⋯


for (int tracker = 0; tracker < score.size(); tracker++) {

looks like it will be the same kind of thing, since you are measuring score and looping over each valid index. Did you really mean to iterate over the collection? Hmm, no, because you are changing the collection. What is this thing actually doing? Code should not be a puzzle to solve. You insert at the current position, and then you erase nothing…I think that line must be a bug, and you meant to erase the last element. { update: after running, I see that this causes a crash rather than doing nothing. “The iterator pos must be valid and dereferenceable. Thus the end() iterator (which is valid, but is not dereferencable) cannot be used as a value for pos” }

Meanwhile, you are searching for the first position where some condition holds, and using that location, then quitting. So the loop is really just a search.

Solution to the puzzle: You are inserting in sorted order, and trimming the lowest to keep the same number of elements!

Now how can you write that to not be a puzzle? It should say “find the position in the sorted list”, then use that position to insert.

The algorithm for that is std::lower_bound. Trimming the end is more efficient if you do that first. So:

score.pop_back();
const auto pos= std::upper_bound (score.begin(), score.end(), currentScore);
score.insert (pos, currentScore);

---

void playAgain() { // is there a better way of doing this? 

Yes. The stuff involved in a game (as opposed to the persistent state like the high scores and settable options) should be in a class. So, when the game is done you destroy the used game and construct a new one. That will be done naturally if the game object is a local variable in the scope of the loop.

---

srand(time(NULL));

Forget that the legacy NULL macro ever existed. Don’t use it — ever. ⧺ES.47: Use nullptr rather than 0 or NULL

But, srand is a poor random generator and the ISO standard even states

Use of rand therefore continues to be non-portable, with unpredictable and oft-questionable quality and performance.

although they have not gone so far as to deprecate it. It’s part of the C library compatibility.

---

system("PAUSE");

What’s that? Non-portable, but what is the purpose of pausing when you quit?

---

Advice

I suggest you start by preparing a rough architecture of the program, showing the needed information and how it is arranged into different classes. Classes are different because of different lifetimes, and single responsibility.

Become familiar with the standard library.

From your comments, it seems like you already have some sense of good coding involving repetition or brute force. Keep it up!

Answer 2

Well, you follow the letter of the law for the first two requirements, as you are using a single god-object containing everything, and it's defined over multiple classes, which are only ever used to inherit for building the full class.
That's probably not what your teacher wanted, but who knows.

Abstain from importing wholesale huge open-ended namespaces like std into the global scope. Read "Why is “using namespace std” considered bad practice?" for the reasons.

playAgain() and playGame() are mutually recursive without break-condition. So, how to stop? Waiting until the stack overflows is not a solution!

Do you know switch-statements? Far preferable to long chains of if-Statements where applicable.

If you want to output a sngle character with iostreams, consider using a character-literal instead a short string-literal. Likely slightly more efficient.

Consider making your playfield a donut, like the classic snake-game uses. Next, save it as a matrix (including walls, snake-parts and food), and you can use that to easily test collisions, eating, and placing more food. That matrix-class (dynamically sized or not) would make a useful class for more than playing-fields…

---

So, you need a 2d-matrix for the playing-field, a struct for coordinates, a class representing a player (score, lives, name, alive, snake-head-coordinates, snake-tail-coordinates (rest of snake seen on the board), snake-length, snake-real-length), maybe derived from that a human player and later a (probably pretty dumb) ai player.
Last, you can have a class for the game that connects everything together.

That seems to better follow both the letter and the spirit of your instructions.