4
\$\begingroup\$

I have made a console game using c++ without using OOP. The code works, but is it good that way or should I have used OOP?

#include <iostream>
#include <utility>
#include <vector>
#include <Windows.h>
#include <ctime>

enum direction {
    STOP, UP, DOWN, RIGHT, LEFT
};

const unsigned short width{ 50 }, height{ 20 };
unsigned short xFruit{}, yFruit{}, xTail{}, yTail{}, score{};
bool lost{ false };
std::vector<std::pair<unsigned short, unsigned short>> snake;
direction dir{ STOP };

void startGame();
void draw();
void input();
void movement();
void eatFruit();
void generateFruit();
void endGame();

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

int main()
{
    startGame();
    while (!lost) {
        srand(static_cast<unsigned int>(time(0)));
        draw();
        input();
        movement();
        eatFruit();
        if (dir == UP || dir == DOWN)
            // To slow the vertical movement
            Sleep(40); 
        else
            // To slow the horizontal movement
            Sleep(20);
    }
    endGame();
    return 0;
}

void startGame() {
    srand(static_cast<unsigned int>(time(0)));
    // Start at the center
    snake.emplace_back(std::make_pair(width / 2, height / 2)); 
    generateFruit();
}

void draw() {
    /*
    * '#' the outer frame
    * '@' the head
    * 'o' the body
    * '*' the fruit 
    */
    ClearScreen();
    for (unsigned short y{ 1 }; y <= height; y++) {
        for (unsigned short x{ 1 }; x <= width; x++) {
            if (x == 1 || x == width)
                std::cout << '#';
            else if (y == 1 || y == height)
                std::cout << '#';
            else if (x == snake.at(0).first && y == snake.at(0).second)
                std::cout << '@';
            else if (std::find(snake.begin() + 1, snake.end(), std::make_pair(x, y)) != snake.end())
                std::cout << 'o';
            else if (x == xFruit && y == yFruit)
                std::cout << '*';
            else
                std::cout << ' ';
        }
        std::cout << std::endl;
    }
    std::cout << "Use arrow keys to move the snake" << std::endl;
    std::cout << "Score: " << score << std::endl;
}

void input() {
    if (GetAsyncKeyState(VK_UP) && dir != DOWN)
        dir = UP;
    else if (GetAsyncKeyState(VK_RIGHT) && dir != LEFT)
        dir = RIGHT;
    else if (GetAsyncKeyState(VK_DOWN) && dir != UP)
        dir = DOWN;
    else if (GetAsyncKeyState(VK_LEFT) && dir != RIGHT)
        dir = LEFT;
}

void movement() {
    // Move each part's place of the body to the previous part's place starting from the end of the snake to the second part 
    for (unsigned int part{snake.size() - 1}; part > 0; part--) {
        snake.at(part) = snake.at(part - 1);
    }
    if (dir == UP) {
        snake.at(0).second--;
        // if the snake hit the top wall, move to the bottom
        if (snake.at(0).second <= 1)
            snake.at(0).second = height - 1;
    }
    else if (dir == DOWN) {
        snake.at(0).second++;
        // if the snake hit the bottom wall, move to the top
        if (snake.at(0).second >= height)
            snake.at(0).second = 2;
    }
    else if (dir == RIGHT) {
        snake.at(0).first++;
        // if the snake hit the right wall, move to the left
        if (snake.at(0).first >= width)
            snake.at(0).first = 2;
    }
    else if (dir == LEFT) {
        snake.at(0).first--;
        // if the snake hit the left wall, move to the right
        if (snake.at(0).first <= 1)
            snake.at(0).first = width - 1;
    }
    // If the snake hit itself
    if (std::find(snake.begin()+1, snake.end(), snake.at(0)) != snake.end())
        lost = true;

}

void eatFruit() {
    if (snake.at(0).first == xFruit && snake.at(0).second == yFruit) {
        generateFruit();
        score++;
        // create new part of the snake
        if (dir == UP) {
            xTail = snake.back().first;
            yTail = snake.back().second + 1;
        }
        else if (dir == DOWN) {
            xTail = snake.back().first;
            yTail = snake.back().second - 1;
        }
        else if (dir == RIGHT) {
            xTail = snake.back().first - 1;
            yTail = snake.back().second;
        }
        else if (dir == LEFT) {
            xTail = snake.back().first + 1;
            yTail = snake.back().second;
        }
        snake.emplace_back(std::make_pair(xTail, yTail));
    }
}

void generateFruit() {
    xFruit = rand() % (width - 3) + 2;
    yFruit = rand() % (height - 3) + 2;
    // To check if the fruit appeared behind the snake
    if (std::find(snake.begin(), snake.end(), std::make_pair(xFruit, yFruit)) != snake.end())
        generateFruit();
}

void endGame() {
    std::cout << "You lost, Score: " << score << std::endl;
    std::cin.get();
}
\$\endgroup\$
  • 2
    \$\begingroup\$ Hi, and welcome to the site. It would be helpful if you could clarify how a "snake game" is supposed to work. \$\endgroup\$ – Donald.McLean Jul 25 at 15:14
  • 1
    \$\begingroup\$ The program does not build on Visual Studio 2015 x64. The problem is the first line in movement(): part is declared as unsigned int, however, snake.size()-1 is std::size_t. The brace initialization forbids the conversion. It builds if you declare part as std::size_t. \$\endgroup\$ – pschill Jul 26 at 8:06
3
\$\begingroup\$

Only call srand once. Continually reseeding it can result in non-random random numbers.

The statements in ClearScreen should be on separate lines. With them all spread out like that it is hard to see what it is doing (which doesn't seem to actually clear the screen, just move the cursor to the top left of the console).

In main, the if statements should have curly braces because the two statement bodies are on multiple lines. This can avoid future bugs when adding code. It can also be condensed into one statement with the ternary operator. And the two constants (20 and 40) can be turned into named constants so that no commentary would be necessary.

In draw, the checks for x or y equaling 1, height, or width can be removed and the std::cout << '#'; being placed outside the appropriate loop (modified to print the entire line of '#' characters for the top and bottom row). Then you can create a local variable char ch to hold the character that you want to print, and have only one std::cout << ch instead of repeating them.

In eatFruit, you don't need to determine the position of the new body segment. Just increase the length of snake (copying in the value of the snake's last segment, since you'll draw again before you move), and the following call to move will update it when the previous end moves off of that spot.

generateFruit can use a do/while loop instead of recursion.

You might also consider putting in the pause right after the draw, rather than after the input. This may make the snake a bit more responsive to user input.

\$\endgroup\$
  • \$\begingroup\$ ClearScreen doesn't technically clear the screen, it moves the cursor to the top left and rewrites the previously written frame. Thanks for your advice. \$\endgroup\$ – Mina Yossry Jul 25 at 6:53
2
\$\begingroup\$

This is in addition to the answer provided by @1201ProgramAlarm, everything in their review is correct.

I have made a console game using c++ without using OOP. The game works perfectly, but is it good that way or should I have used OOP?

Bug
The program doesn't work perfectly, the snake is supposed to die if it hits a wall. In this implementation the snake re-enters on the opposite side of the screen.

Opinion
The second part of the question, if it should be Object Oriented or not is off-topic for Code Review because it is primarily opinion based. The code is also not completely free of objects, the std::vector<> container class is used, and iterators are used.

Global Variable
Generally global variables are considered a bad practice. In larger programs with multiple source files it is very difficult to write correct code, debug and maintain code using global variables. Finding where a variable value is altered becomes a huge task. A second problem with global variables in a multi-file program is that global variables are global in scope and can lead to duplicate definitions at link time. In an object oriented program all of these variables could have been global within the object but the scope would still be limited to the object itself.

In non-object oriented programming it might be better to declare the variables in functions or procedures and pass the values into sub functions.

The variables xtail and ytail should be declared in the function void eatFruit() since they are not used anywhere else in the program.

Variable Declarations and Performance
The code is primarily using unsigned short variables and this will affect performance because it is not the default word size. The default word size in C++ is int or unsigned int. Generally the computer instructions that are generated by the code will be using the default word size. The short type is generally used within objects to save space, which does not seem to be necessary in this case.

Use Vertical Spacing to Make the Code more Readable
To make code easier to maintain it would be better to have one variable declared per statement and each declaration on it's own line. Rather than saving vertical space, use vertical space to make the code easier to read and update:

const unsigned short width = 50;
const unsigned short height = 20;

unsigned short xFruit;
unsigned short yFruit;
unsigned short score = 0;

void ClearScreen()
{
    // Function which cleans the screen without flickering
    COORD cursorPosition;

    cursorPosition.X = 0;   
    cursorPosition.Y = 0;

    SetConsoleCursorPosition(GetStdHandle(STD_OUTPUT_HANDLE), cursorPosition);
}
\$\endgroup\$
  • 1
    \$\begingroup\$ In this case, I feel that that "bug" is intended. There is specific code in movement to handle this. \$\endgroup\$ – Alex F Jul 26 at 1:54

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.