3
\$\begingroup\$

I'm a beginner programmer and i'm looking for interesting projects to improve my low skills. I decided to spend one evening on simple snake game in console. I made it with the help of YouTube tutorial. But then i have modified some parts of the code and commented it.It seems like it's still have a lot of problems. It will be great if someone explain me how to improve it :)

Code:

#include <iostream>
#include <stdlib.h>
#include <windows.h>
#include <conio.h>
#include <ctime>
using namespace std;

// Variables and arrays declaration
bool gameOver;
bool invalidCoord;
const int width = 20;
const int height = 20;
int x, y, fruitX, fruitY, score;
int tailX[100], tailY[100];
int tailLength;

enum Direction { STOP = 0, LEFT, RIGHT, UP, DOWN};
Direction dir;

void ClearScreen()
{   
    // Function which cleans the screen without flickering
    COORD cursorPosition;   cursorPosition.X = 0;   cursorPosition.Y = 0;   SetConsoleCursorPosition(GetStdHandle(STD_OUTPUT_HANDLE), cursorPosition);
}

void Setup() 
{   // Initialise variables
    gameOver = false;
    dir = STOP;
    srand(time(0));
    x = width / 2;
    y = height / 2;
    fruitX = rand() % width;
    fruitY = rand() % height;
    score = 0;

}

void Draw() // Drawing playing field, snake and fruits
{
    ClearScreen();

    // Draws top border
    for (int i = 0; i < width + 2; i++)
        cout << '-';
    cout << endl;

    for (int i = 0; i < height; i++)
    {
        for (int k = 0; k < width; k++)
        {   
            // Left border
            if (k == 0)
                cout << '|';
            // Snake's head
            if (i == y && k == x)
                cout << '@';
            // Fruit
            else if (i == fruitY && k == fruitX)
                cout << '*';

            else
            {   
                // Checks if there is a tail block with appropriate coordinates and draws it 
                bool printTail = false;
                for (int j = 0; j < tailLength; j++)
                {
                    if (tailX[j] == k && tailY[j] == i)
                    {
                        cout << 'o';
                        printTail = true;
                    }
                }
                // Draws blank space if there is nothing to display
                if (!printTail)
                    cout << ' ';
            }

            // Right border
            if (k == width - 1)
                cout << '|';

        }
        cout << endl;
    }

    // Draws bottom border
    for (int i = 0; i < width + 2; i++)
        cout << '-';
    cout << endl;

    // Displays player's score
    cout << endl;
    cout << "Score: " << score << endl;

}
void Input()
{   
    // Changes snake's direction depending on the button pressed and doesn't allow player to change direction in invalid way 
    if (_kbhit())
    {
        switch (_getch())
        {
        case 'w':
            if (dir != DOWN)
                dir = UP;
            break;
        case 'a':
            if (dir != RIGHT)
                dir = LEFT;
            break;
        case 's':
            if (dir != UP)
                dir = DOWN;
            break;
        case 'd':
            if (dir != LEFT)
                dir = RIGHT;
            break;
        case 'k':
            gameOver = true;
            break;
        }

    }
}

void Logic() 
{   
    // Tail logic. Every new eteration we remember previous position of the head and save it to prevX, prevY.
    // Then we update array with snake's parts positions (change first numbers in arrays tailX, tailY to a new head coordinates).
    // And after that for each number in arrays except the first ones we make some changes.
    // Save tailX[i], tailY[i] to prevX2, prevY2 and equate tailX[i], tailY[i] to prevX, prevY.
    // And equate prevX, prevY to prevX2, prevY2.
    // Then change rest of the arrays in the same way.

    int prevX = tailX[0];
    int prevY = tailY[0];
    int prevX2, prevY2;
    tailX[0] = x;
    tailY[0] = y;

    for (int i = 1; i < tailLength; i++)
    {
        prevX2 = tailX[i];
        prevY2 = tailY[i];
        tailX[i] = prevX;
        tailY[i] = prevY;
        prevX = prevX2;
        prevY = prevY2;
    }
    // Changes snake's head coordinates depending on a direction
    switch (dir)
    {
    case LEFT:
        x--;
        break;
    case RIGHT:
        x++;
        break;
    case UP:
        y--;
        break;
    case DOWN:
        y++;
        break;
    }

    // Detects collision with a tail
    for (int i = 0; i < tailLength; i++)
        if (tailX[i] == x && tailY[i] == y)
            gameOver = true;

    // Detects collision with a fruit
    if (x == fruitX && y == fruitY)
    {
        score += 1;
        fruitX = rand() % width;
        fruitY = rand() % height;
        // Generate new fruit position if it consides with snake's tail position 
        for (int i = 0; i < tailLength; )
        {   
            invalidCoord = false;
            if (tailX[i] == fruitX && tailY[i] == fruitY) {
                invalidCoord = true;
                fruitX = rand() % width;
                fruitY = rand() % height;
                break;
            }
            if (!invalidCoord)
                i++;
        }
        tailLength++;
    }

    // Changes snake position if it goes through the wall
    if (y >= height)
        y = 0;
    else if (y < 0)
        y = height - 1;
    if (x >= width)
        x = 0;
    else if (x < 0)
        x = width - 1;
}

int main()
{
    Setup();
    while (!gameOver) // Game mainloop 
    {
        Draw();
        if (dir == UP || DOWN)
            Sleep(25); // Helps to equate vertical snake movement speed and horizontal speed
        Sleep(40);
        Input();
        Logic();

    }

    return 0;
}
\$\endgroup\$
4
  • 3
    \$\begingroup\$ i have modified some parts of the code So how much of this was actually written by you? \$\endgroup\$
    – yuri
    Commented Jun 9, 2019 at 17:11
  • 1
    \$\begingroup\$ @yuri I wrote it by myself from the top to the bottom, i just use vital conception of game logic \$\endgroup\$ Commented Jun 11, 2019 at 10:09
  • 1
    \$\begingroup\$ "It seems like it's still have a lot of problems." What kind of problems? Because if the code doesn't work as it should, it will be closed as off-topic. If you mean "problems" as in code style etc, you should specify it to make it clear. \$\endgroup\$
    – IEatBagels
    Commented Jun 25, 2019 at 17:52
  • \$\begingroup\$ @IEatBagels Game works correctly, but the main problem is that fruit can spawn in the snake‘s tail, i tried to solve it by making a for loop which generate new coordinates for fruit if they equal to any coordinates in tailX, tailY Arrays, but there are a still chance that new coordinates will be inappropriate again \$\endgroup\$ Commented Jun 27, 2019 at 8:20

1 Answer 1

3
\$\begingroup\$

General Observations

It isn't clear why you chose C++ over C: this is primarily a C program except that it uses C++ input and output. There is no use of C++ features.

When you do use C header files in C++ such as stdlib.h there are generally C++ versions such as cstdlib.

Avoid using namespace std;

If you are coding professionally you probably should get out of the habit of using the using namespace std; statement. The code will more clearly define where cout and other identifiers are coming from (std::cin, std::cout). As you start using namespaces in your code it is better to identify where each function comes from because there may be function name collisions from different namespaces. The identifiercout you may override within your own classes, and you may override the operator << in your own classes as well. This stack overflow question discusses this in more detail.

Avoid Global Variables

It is very difficult to read, write, debug and maintain programs that use global variables. Global variables can be modified by any function within the program and therefore require each function to be examined before making changes in the code. In C and C++ global variables impact the namespace and they can cause linking errors if they are defined in multiple files. The answers in this Stack Overflow question provide a fuller explanation.

If you must use global variables in a file, declare them as static so that they only impact the namespace of that file; that will prevent some of the linking errors mentioned above.

Declare the Variables as Needed

In the original version of C back in the 1970s and 1980s variables had to be declared at the top of the function. That is no longer the case, and a recommended programming practice to declare the variable as needed. In C the language doesn't provide a default initialization of the variable so variables should be initialized as part of the declaration. For readability and maintainability each variable should be declared and initialized on its own line.

Prefer C++ Random Number Generators

The C++ libraries provide better random number generators than the C library functions srand() and rand().

The following code is from the top answer of this Stack Overflow question. This code provides a C++ random number generator that is better than rand().

#include <random>
#include <iostream>

int main()
{
    std::random_device dev;
    std::mt19937 rng(dev());
    std::uniform_int_distribution<std::mt19937::result_type> dist6(1,6); // distribution in range [1, 6]

    std::cout << dist6(rng) << std::endl;
}

Enum Initialization

Generally there is no need to use a numeric initialization of enum values. The only reason to do this is if you are depending on the enum to be a specific value, and that has a smell to it.

By default the first enum value will be zero so this isn't needed in any case.

enum Direction { STOP = 0, LEFT, RIGHT, UP, DOWN };

Limit the Length of the Lines

This code is too wide; that makes it hard to read, and maintain the code. Each statement should be on a separate line so that the code is easier to read and maintain.

void ClearScreen()
{
    // Function which cleans the screen without flickering
    COORD cursorPosition;   cursorPosition.X = 0;   cursorPosition.Y = 0;   SetConsoleCursorPosition(GetStdHandle(STD_OUTPUT_HANDLE), cursorPosition);
}

Versus

void ClearScreen()
{
    // Function which cleans the screen without flickering
    COORD cursorPosition;
    cursorPosition.X = 0;
    cursorPosition.Y = 0;
    SetConsoleCursorPosition(GetStdHandle(STD_OUTPUT_HANDLE), cursorPosition);
}

Please note that the last statement is definitely long enough to be on a line by itself in any case.

Problematic Code

This if statement in main() barely compiles and it may not do what you want it to.

        if (dir == UP || DOWN)

It should be

        if (dir == UP || dir == DOWN)

Complexity

The function Logic() is too complex (does too much). It would be better to break the code up into smaller functions with only one purpose.

There is also a programming principle called the Single Responsibility Principle that applies here. The Single Responsibility Principle states:

that every module, class, or function should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by that module, class or function.

The function is almost 80 lines long, this is larger than a single screen in any IDE or editor. A general best practice in programming is to keep a function to less than one screen, that makes the code easier to understand and easier to maintain.

\$\endgroup\$
4
  • \$\begingroup\$ Even in C, variables had to be declared at the top of function only in extremely early versions of C (before K&R first edition). Well before the 1980s, it was changed to support declaring variables at the top of any scope, immediately after the opening brace. \$\endgroup\$ Commented Jan 16, 2023 at 16:11
  • \$\begingroup\$ @TobySpeight K&R first edition required variable declarations immediately after opening braces, I still have a copy and I programmed in it for 10 years on Sun OS 1.1 - Sun OS 4 (BSD) and VAX 11 C. 5 of those years was writing C compilers in C. \$\endgroup\$
    – pacmaninbw
    Commented Jan 19, 2023 at 15:07
  • \$\begingroup\$ Yes, that's right. After any opening brace (of a compound statement or function body). You have to go back further than that to find a version of C where you couldn't declare variables in inner scopes. \$\endgroup\$ Commented Jan 19, 2023 at 16:38
  • \$\begingroup\$ @TobySpeight Will change that in the future. \$\endgroup\$
    – pacmaninbw
    Commented Jan 19, 2023 at 16:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.