Snake Game C++ Improvements?

Question

Is there anything you would suggest to improve this code? In particular, any code to make the snake move more smoothly and make the cursor disappear.

#include <iostream.h>
#include <conio.h>
#include <stdlib.h>
#include <dos.h>

unsigned long int score, high_score = 0;

const int no_rows = 22, no_columns = 77, head_position = 0;

char choice, token, token_food, key_press, direction_head, overlay;

char board[no_rows][no_columns];

unsigned int i, j, k, snake_length, grow, crash;

struct position
{
    int row;
    int column;
} head_map[no_rows * no_columns], food, temp1, temp2;

void Initialize()
{
start:
    clrscr();

    cout << "\t\t\t\tWELCOME TO SNAKE GAME\n\n\n";
    cout << "Choose the Body of the Snake(Enter a symbol):\n";
    cin >> token;
    cout << "\nChoose the Food of the Snake(Enter a symbol):\n";
    cin >> token_food;

    if(token_food == token)
    {
        clrscr();
        cout << "\nDon't use the same symbols for food and body of the snake!!!\n\n\n";
        goto start;
    }

    cout << "\n\nUP\t-\tW\nDOWN\t-\tS\nRIGHT\t-\tD\nLEFT\t-\tA\n\n\nARE YOU
         READY(Y / N) ?? \n";
         cin >> choice;

    for (i = 0; i < no_rows; i++)
    {
        for (j = 0; j < no_columns; j++)
            board[i][j] = ' ';
    }

    head_map[0].row = no_rows / 2;
    head_map[0].column = no_columns / 2;

    board[head_map[0].row][head_map[0].column] = token;

    direction_head = 'R';
    snake_length = 1;

    grow = crash = score = 0;
}

void Food()
{
    do
    {
        food.row = rand() % no_rows;
        food.column = rand() % no_columns;
    }
    while(board[food.row][food.column] == token);

    board[food.row][food.column] = token_food;
}

void Display()
{
    clrscr();

    for(k = 0; k < (no_columns + 2); k++)
        cout << "-";

    cout << endl;

    for(i = 0; i < no_rows; i++)
    {
        cout << "|";
        for(j = 0; j < no_columns; j++)
            cout << board[i][j];
        cout << "|" << endl;
    }

    for(k = 0; k < (no_columns + 2); k++)
        cout << "-";

    cout << "\t\t\t\t\t  SCORE :" << score;
}

void Input()
{
    if(kbhit())
    {
        key_press = getch();

        if((key_press == 'd' || key_press == 'D') && (key_press != 'a' || key_press != 'A') && (direction_head != 'L'))
            direction_head = 'R';

        if((key_press == 's' || key_press == 'S') && (key_press != 'w' || key_press != 'W') && (direction_head != 'U'))
            direction_head = 'D';

        if((key_press == 'a' || key_press == 'A') && (key_press != 'd' || key_press != 'D') && (direction_head != 'R'))
            direction_head = 'L';

        if((key_press == 'w' || key_press == 'W') && (key_press != 's' || key_press != 'S') && (direction_head != 'D'))
            direction_head = 'U';

        if(key_press == 'p' || key_press == 'P')
            delay(5000);
    }

    temp1 = head_map[head_position];
    board[head_map[head_position].row][head_map[head_position].column] = ' ';

    switch(direction_head)
    {
    case 'R':
    {
        if(head_map[head_position].column == no_columns)
            crash = 1;

        overlay = board[head_map[head_position].row][++head_map[head_position].column];

        break;
    }

    case 'L':
    {
        if(head_map[head_position].column == 0)
            crash = 1;

        overlay = board[head_map[head_position].row][--head_map[head_position].column];

        break;
    }

    case 'D':
    {
        if(head_map[head_position].row == no_rows)
            crash = 1;

        overlay = board[++head_map[head_position].row][head_map[head_position].column];

        break;
    }

    case 'U':
    {
        if(head_map[head_position].row == 0)
            crash = 1;

        overlay = board[--head_map[head_position].row][head_map[head_position].column];

        break;
    }

    default:
        break;
    }
}

void Move()
{
    board[head_map[head_position].row][head_map[head_position].column] = token;

    for(i = 1; i < snake_length; i++)
    {
        temp2 = head_map[i];
        head_map[i] = temp1;

        board[head_map[i].row][head_map[i].column] = token;
        board[temp2.row][temp2.column] = ' ';

        temp1 = temp2;
    }

    if(grow == 1)
    {
        head_map[i] = temp1;
        board[head_map[i].row][head_map[i].column] = token;

        ++snake_length;
        score += 10;
        Food();
        grow = 0;
    }
}

void Change()
{
    if(overlay == token)
        crash = 1;

    if(overlay == token_food)
        grow = 1;
}

void Game_over()
{
    getch();
    clrscr();

    cout << "\n\n\n\n\n\n\n\n\n\n\t\t\t\tGAME OVER!!!\n\t\t\t\tFINAL SCORE: " << score;

    if(score > high_score)
    {
        high_score = score;
        cout << "\n\t\t\tYOU BEAT THE HIGH SCORE!!!";
    }

    else
    {
        if(score == high_score)
            cout << "\n\t\t\tYOU TIED THE HIGH SCORE!!!";

        else
            cout << "\n\t\t\tHIGH SCORE: " << high_score;
    }
    getch();
    clrscr();

    cout << "\n\n\n\n\n\n\n\n\n\n\n\t\t\tDO YOU WANT TO CONTINUE(Y/N)??\n\t\t\t\t\t";
    cin >> choice;

    getch();
}

int main()
{
    do
    {
        Initialize();
        Food();
        Display();

        do
        {
            Input();
            Change();

            if(crash == 1)
                break;

            Move();
            delay(200);

            Display();

        }
        while(crash != 1);

        Game_over();

    }
    while(choice == 'Y' || choice == 'y');

    return 0;
}

Answer 1

Making the code more modern

Your code looks very old. This is because the <iostream.h> header in line 1 is not used anymore since about the year 1998. Same for the <dos.h> and <conio.h> headers.

To get your program to compile with a modern C++ compiler, I had to replace the first paragraph of your code with this:

#include <iostream>

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

extern "C" {
    void clrscr();
    bool kbhit();
    void delay(int);
    int getch();
}

I only did this change to make your code valid for my compiler. It won't run after these changes. Therefore you should not apply these changes to your code. Just leave your code as it is. Don't be surprised though when you try to run your code in a more modern environment, as this won't work.

Putting text on the screen

I noticed that you use long sequences of \n and \t to place the text on the screen. Since you are using the clrscr function, I suppose that the function gotoxy is also defined. If so, you can replace this code:

    clrscr();

    cout << "\n\n\n\n\n\n\n\n\n\n\t\t\t\tGAME OVER!!!\n\t\t\t\tFINAL SCORE: " << score;

with this code:

    clrscr();

    gotoxy(32, 10);
    cout << "GAME OVER!!!";
    gotoxy(32, 11);
    cout << "FINAL SCORE: " << score;

This code takes more vertical space than before, but there is no need anymore to count the number of \n characters in the string.

Input methods

You are using two fundamentally different input methods:

1. cin >> choice, which reads a character, but the program only sees this character after Enter has been pressed. This is not suitable for a snake game.

2. getch(), which reads a single key without requiring the Enter key. This function works closely together with kbhit().

You should not mix these two, at least not in the same phase of the game. There is the dialog phase ("play again?"), which should use cin >> choice, and there is the playing phase, which should use kbhit() and getch() (in this order).

Reducing the scope of variables

Your code declares (among others) these variables:

unsigned int i, j, k;

These variables are used later by pieces of code that are completely unrelated to each other. Therefore it doesn't make sense that these unrelated code pieces use the same variables. One of these pieces is:

    for (i = 0; i < no_rows; i++)
    {
        for (j = 0; j < no_columns; j++)
            board[i][j] = ' ';
    }

After the opening parenthesis of each for loop, you should declare the variable, which is then in scope for the rest of the for loop. The changed code is:

    for (unsigned int i = 0; i < no_rows; i++)
    {
        for (unsigned int j = 0; j < no_columns; j++)
            board[i][j] = ' ';
    }

When you do that in the other for loops as well, there will be a compile error:

        head_map[i] = temp1;
        board[head_map[i].row][head_map[i].column] = token;

This compile error means that your code is somewhat unusual. You used i in a loop, and usually that variable is not needed after the loop. Not so in this case.

When the for loop is finished, i will be the same as snake_length. Therefore you can replace the code with the very similar:

        head_map[snake_length] = temp1;
        board[head_map[snake_length].row][head_map[snake_length].column] = token;

This makes the intention of the code a bit clearer, since for experienced programmers the variable name i means a variable that changes its value often, such as in your loops that fill the board with spaces. That name i would be misleading here, since the code handles the tail of the snake. The expression head_map[snake_length] expresses this more clearly than the expression head_map[i].

Redundant conditions

Further down, you have this code:

        if((key_press == 'w' || key_press == 'W') && (key_press != 's' || key_press != 'S') && (direction_head != 'D'))

This code is redundant. If the pressed key is 'w' or 'W', it cannot be 's' at the same time. Therefore you don't need to check for 's' at all.

Furthermore, the expression key_press != 's' || key_press != 'S' will always be true. There are 3 cases:

1. key_press != 's': the first condition is true, therefore the whole expression is true.
2. key_press != 'S': the second condition is true, therefore the whole expression is true.
3. any other key: both conditions are true, therefore the whole expression is true.

Therefore, the simplified code is:

        if((key_press == 'w' || key_press == 'W') && (true) && (direction_head != 'D'))

This can be further simplified to:

        if((key_press == 'w' || key_press == 'W') && direction_head != 'D')

Pausing the game

        if(key_press == 'p' || key_press == 'P')
            delay(5000);

This looks wrong. When I press the 'p' key, I expect the game to pause until I explicitly continue it by pressing 'p' again. Waiting 5 seconds is something entirely different.

To implement the pause correctly, you should define a global variable:

bool paused = false;

After that, adjust the code from above:

    if(kbhit())
    {
        ...

        if(key_press == 'p' || key_press == 'P')
            paused = !paused;
    }

    if (paused)
        return;

Since the second part of the Input function does not deal with the input at all but instead moves the snake, that part is skipped as long as the game is paused.

Final words

Your code is structured well, especially since the code of the main function gives a rough overview over the whole game flow, just as it should.

You named the functions well, which makes the main function easy to grasp.

There are many more things that can be said about your code, but they are not urgent. Getting a few ideas and thinking about them is easier than getting a hundred tips at once.

Answer 2

Code improvements:

1. Do not use using namespace std it saves you some time typing the code, but when other namespace get involved later on its a mess to debug this code. Furthermore I personally find it more aesthetic to see from which namespace a functions "comes from".
2. Consider using a function that checks wether the user-input is valid.

   cout << "Choose the Body of the Snake(Enter a symbol):\n";
   cin >> token;
   

   This, for instance is dangerous. What if I (the user) enter a non ASCII symbol like an emoji. 😀 Or even worse - a character that isn't even printable.

Answer 3

Your main bottleneck is, of course, using the console for drawing the frame. I suggest you to stop using std::endl in your Display() function, and write a new line character instead.

I believe this only helps you, though, if you turn off automatic flushing usingstd::ios::sync_with_stdio. You have to see yourself if this is an improvement on your machine anyway.

Actually there are other problems I see in this code you should care about; quality, and not performance-wise. But I wouldn't address those now, since you didn't ask for it.