ASCII-based snake game in C++

Question

I have recently finished developing my ASCII based snake game. As a newbie to C++ I would appreciate it if I was given feedback as to how I could improve my code when it comes to efficiency and readability.

#include <iostream>
#include <cstdlib>
#include <conio.h>
#include <vector>

using std::cout; using std::cin; using std::endl;

const int width = 20;
const int height = 20;
bool gameState = true;
int score;

struct fruitClass {
    int x;
    int y;
};

struct Node {
    int x;
    int y;
    struct Node *next;
};

Node *newNode(int x, int y) {
    Node *link = new(Node);
    link->x = x;
    link->y = y;
    return link;
}

void rotateList(Node *head, int x, int y) {
    int i;
    Node *current = head;
    int tempX[2], tempY[2];
    for (i = 0; current != nullptr; i++) {

        if (i == 0) {
            tempX[0] = current->x;
            tempY[0] = current->y;
            current->x = x;
            current->y = y;
        }
        else {
            tempX[(i % 2 == 0) ? 0 : 1] = current->x;
            tempY[(i % 2 == 0) ? 0 : 1] = current->y;
            current->x = tempX[(i % 2 == 0) ? 1 : 0];
            current->y = tempY[(i % 2 == 0) ? 1 : 0];
        }

        current = current->next;
    }

}


void append(Node *&head, Node *&link) {
    Node *ptr;
    ptr = head;

    head = link;
    head->next = ptr;
}

Node *head;
fruitClass fruit;

class snakeClass {
private:
    int posX;
    int posY;
    int tailLen = 0;
    enum eDirection{STOP = 0, UP, LEFT, RIGHT, DOWN};
    eDirection dir;
public:
    void init () {
        //Initialise snake's position
        posX = width/2;
        posY = height/2;

        //Initialise first tail node
        head = newNode(posX, posY);

        //Spawn fruit
        fruit.x = rand() % width;
        fruit.y = rand() % height;

        //Initialise score
        score = 0;

        //Initialise direction
        dir = STOP;
    }
    void draw() {
        system("cls");

        //Draw top of playing field
        for (int i = 0; i < width+2; i++)
            cout << '#';
        cout << '\n';

        //Main draw
        for (int i = 0; i < height; i++) {
            for (int j = 0; j < width; j++) {


                bool found = false;

                //Draw left of playing field
                if (j == 0)
                    cout << '#';

                //Draw snake's head
                if (i == posY && j == posX) {
                    cout << 'O';
                    found = true;
                }

                Node *current = head;

                //Draw snake's body by extracting data from a linked list
                while (current != nullptr && found != true) {
                    if (i == current->y && j == current->x) {
                        cout << 'o';
                        found = true;
                    }
                    current = current->next;
                }

                //If snake's body is not found check to see if there is anything else in the location
                if (!found) {
                    if (i == fruit.y && j == fruit.x)
                        cout << 'F';
                    else
                        cout << ' ';
                }

                //Draw right side of playing field
                if (j == width-1)
                    cout << '#';
            }
            //Add new lines where needed
            if (i != height-1)
                cout << '\n';
        }
        cout << '\n';

        //Draw bottom of playing field
        for (int i = 0; i < width+2; i++)
            cout << '#';
        cout << endl;

        //Score screen
        cout << "Score: " << score << " || Tail len: " << tailLen << '\n';

    }

    void input() {
        //If a button is pushed
        if (kbhit()) {

            //Controls
            switch(getch()) {
            case 'W' : case 'w' :
                if (dir == STOP)
                    dir = UP;
                break;
            case 'A' : case 'a' :
                if (dir == STOP)
                    dir = LEFT;
                else if (dir == UP)
                    dir = LEFT;
                else if (dir == DOWN)
                    dir = RIGHT;
                else if (dir == LEFT)
                    dir = DOWN;
                else if (dir == RIGHT)
                    dir = UP;
                break;
            case 'S' : case 's' :
                if (dir == STOP)
                    dir = DOWN;
                break;
            case 'D' : case 'd' :
                if (dir == STOP)
                    dir = RIGHT;
                else if (dir == UP)
                    dir = RIGHT;
                else if (dir == DOWN)
                    dir = LEFT;
                else if (dir == LEFT)
                    dir = UP;
                else if (dir == RIGHT)
                    dir = DOWN;
                break;
            case 'x' : case 'X' :
                gameState = false;
                break;
            }
        }
    }

    void logic() {
        //Movement and snake tail logic
        switch (dir) {
        case UP :
            //Rotate list rotates the nodes for the snake tail
            rotateList(head, posX, posY);
            posY--;
            break;
        case DOWN :
            //Rotate list rotates the nodes for the snake tail
            rotateList(head, posX, posY);
            posY++;
            break;
        case LEFT :
            //Rotate list rotates the nodes for the snake tail
            rotateList(head, posX, posY);
            posX--;
            break;
        case RIGHT :
            //Rotate list rotates the nodes for the snake tail
            rotateList(head, posX, posY);
            posX++;
            break;
        }
        //Fruit logic (If fruit is picked up)
        if (posX == fruit.x && posY == fruit.y) {
            //Create new snake tail node
            Node *link = newNode(posX, posY);
            append(head, link);

            //Add score
            score += 100;

            //Respawn fruit
            fruit.x = rand() % width;
            fruit.y = rand() % height;

            //Play noise
            cout << '\a' << endl;

            //Update tail length score
            ++tailLen;
        }

        //Game over logic

        //If snake goes out of bounds end game
        if (posX > width-1 || posX < 0 || posY > height-1 || posY < 0)
            gameState = false;

        //If snake touches tail end game (Cycles through linked list nodes to check)
        Node *current = head->next;
        while (current != nullptr) {
            if (posX == current->x && posY == current->y)
                gameState = false;
            current = current->next;
        }
    }


};
int main () {
    //Run all the code
    snakeClass snake;
    snake.init();
    while (gameState) {
        snake.draw();
            snake.input();
            snake.logic();
        }
        return 0;
    }

Answer 1

Oh boy! A snake game. I'm excited, since this was also one of my first games. So let's get to work, shall we?

Keep it short and simple

The biggest concern with your code gets obvious if we just have a look at your post. Your whole code is inside a single file, which makes it hard to navigate. Your file ist "just" 270 lines though, but still, it's a first step to make your code more readable.

Consistency is key

The next concern is missing consistency. You provide a class for your snake, but a struct with C-like object oriented programming Node. That's somewhat arbitrary.

Also, you're calling your struct Node, which makes it easy to disambiguate it from normal variables (which use camelCase), whereas your other class snakeClass starts with a lowercase, and the Class suffix is superflous. Instead, pick a naming convention that immediately shows whether you have a class-like type, a function or a simple variable.

This leads to

 struct Node;
 struct Fruit;
 struct Snake;

Make your functions easy to use, but hard to use wrong

This one is hard to explain, so let's start with newNode:

Node *newNode(int x, int y) {
    Node *link = new(Node);
    link->x = x;
    link->y = y;
    return link;
}

There are several things going on here that can go wrong. For one, you forgot to set link->next. For another, you use new and Node*, which are both not necessary and can lead to memory leaks if you accidentally use newNode:

 void foo(){
     ptr = newNode(x,y);

     if(some_condition) {
         addPtr(ptr);
     } else {
         return;
     }
 }

If some_condition was wrong, we've just leaked memory. Instead, have newNode return a value:

Node newNode(int x, int y) {
    Node node;
    node.x = x;
    node.y = y;
    node.next = nullptr;
    return node;
}

However, at this point we're just constructing a value, which can be done with {} or a constructor:

Node node = {x, y, nullptr};

// or define a constructor:

struct Node {
    Node(int x, int y, Node * next = nullptr) : x(x),y(y),next(next){}

    int x;
    int y;
    Node * next;
}

This makes it tremendously harder to use newNode in a wrong way.

Oh, and you use newHead wrong. Remember snakeClass::init()? If you ever want to play multiple games in a row, you're going to leak memory. More on that later, see "global state" below.

Rotating is easy with the correct data structure

You're Node is essentially a single-linked list, which makes rotating awkward. You can, however, rotate in O(1) if you use the correct data structure: a double-linked list. However, it's not very memory friendly, since you would have to create a new node in every step of your snake.

Still, let's try it instead. First, instead of having next in Node, let's create a simpler variant, SnakePart:

struct SnakePart {
    int x;
    int y;
};

Now, instead of a global head, let's add a function in your snakeClass, called advance, and a std::deque<SnakePart>:

class SnakePart {
    std::deque<SnakePart> parts;

    void advance(int x, int y, bool loose_tail = true) {
        parts.emplace_front({x,y});

        if(loose_tail)
            parts.pop_back();
    }
};

I'm not kidding. That's the whole logic behind your rotateList, written in a double-ended queue. Note that this is still not optimal, since you will usually only change a single element. I guess that a std::vector<SnakePart> with an additional index will be more cache friendly, but that's up to benchmarking.

Either way, now that you have a std container, you can use all algorithms, iterators and other nice things.

Be aware of global state

I get it. It's just Snake. So there's no harm in using global variables, right?

Wrong. Here's a little experiment: let's say you want to save and load all necessary variables for your game, so that a player can save it to disk, and continue later. Can you identify all those variables without scrolling in your code?

Is it likely that you forget one of those variables? Remember, make your functions easy to use, but hard to use wrong. For your game, I recommend you to create a small class Game that handles all this data for you:

class Game {
    Snake snake;
    Fruit fruit;
    Score score;
    Time  time;
  public:
    ...
};

It doesn't need to be a class, it can also be a small struct, but it's usually a good idea to have something that handles the last few bytes. Your snakeClass almost provides this functionality, although several variables are still global instead of local.

That way, you can easily ask the user for a new game:

int main(){
    char answer = 'y';

    do {
        playGame();
        std::cout << "Do you want to play again? y/n: ";
        answer = getAnswer();
    } while(answer != 'n');
}

You can then even provide keys to save/load your game, which helps tremendously with debugging.

Keep it short and simple

We've had this heading already, but it's here for a second time: make your functions shorter, if possible. Remember, code is written for two participants: the human reader, and the compiler. If the compiler doesn't understand your code, it will throw errors and warnings. If the human doesn't understand your code, it will get angry, sad, or stark raving mad.

Your input function is a valid candidate. The functionality of a and d can be split into rotateLeft and rotateRight. Both functions will be short, and can be checked for correctness.

Further remarks

Now follow further remarks on your code, which aren't that important for a language learner, but still provide further feedback and place for improvement.

Bugs and dead code

• your fruit can spawn in your snake. You don't want that to happen.
• rand() % some_number is biased. Use a function from <random> instead
• you use using std::cin, but you never actually use it in your code
• you include <vector>, but you never actually use it

Drawing efficiently

Instead of redrawing the whole screen every time, only repaint the head and the tail of the snake and the score. This will also reduce flickering. You will need a way to move to a (x,y) coordinate, though. This depends on your operating system and the used terminal emulation. I've used

void gotoxy(int x,int y)
{
   COORD cur={x,y};
   SetConsoleCursorPosition(GetStdHandle(STD_OUTPUT_HANDLE),cur);
}

in my own old Snake game, but I'm not sure whether that's best practices, to be honest. My old Snake game used a single-linked list, too, by the way.

Overall

Your code mostly suffers by missing consistency and global variables, but apart from that, it's just the length of the individual functions that tarnish the overall experience. But that's just nitpicking. Keep in mind that you've just completet your very own Snake game. Well done. But keep on pushing and eating those fruits. Good luck!