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;
}
1 Answer 1
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.
-
\$\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\$Toby Speight– Toby Speight2023年01月16日 16:11:38 +00:00Commented 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\$2023年01月19日 15:07:09 +00:00Commented 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\$Toby Speight– Toby Speight2023年01月19日 16:38:49 +00:00Commented Jan 19, 2023 at 16:38
-
\$\begingroup\$ @TobySpeight Will change that in the future. \$\endgroup\$2023年01月19日 16:55:39 +00:00Commented Jan 19, 2023 at 16:55
Explore related questions
See similar questions with these tags.
i have modified some parts of the code
So how much of this was actually written by you? \$\endgroup\$