Return to Answer

• 1

• 17.3k
• 4
• 31
• 89

###Globals with weak names:

These global constants & variable have far too common names to be declared at this scope:

const int width = 60;
const int height = 15;
double diff=0.5;

There is a real name collision chance with width/height if you want to declared local variables with the same names. Those can and should be static members of the Snake class. E.g.:

// Class Snake
private:
 static const int width = 60;
 static const int height = 15;
 int check[width][height];

diff has absolutely no reason being a global. Why didn't you declare it as a member variable in the first place?

###main() looks bad:

I'm going to be honest with you, that main() function looks pretty bad. That goto is jumping to a point before the initialization of the Snake instance (not sure if intentional). That's a complete mess. Replace it with some kind of for or while loop ASAP.

Also, be sure to improve that unusual indentation. if/else chains should be indented normally, like this:

if (something)
{
}
else if (some_other_thing)
{
}
else if (yet_another)
{
}

###Your program invokes Undefined Behavior (UB):

You are calling main() from Snake::game_over(). This is an ill formed program which your compiler is probably allowing. You might be getting a warning out of that though. Always check warnings. Compiling with Warnings as Errors might also be a sensible approach in some cases, in special to help disciplining yourself into paying attention to them.

###system("xyz") and friends are exploitable:

By calling system() you ask for the Operating System to run an external program, which might not be a trusted one. Imagine a hacker replaces the cls utility with some malware. Your program would be firing this malware program in the host machine. So be aware of this problem and never use it in a scenario where you would actually care about security. system("cls"), system("pause") are also not portable (pause and cls are Windows utilities), though that doesn't seem to be a problem since you are already using other Windows specific things in your program.

###Magic numbers:

You have a lot of magic numbers and hardcoded constants laying around, such as menu indexes and quite a few char(233) and the like. Those should be turned into named constants. If I don't remember my ASCII table by memory, it is pretty hard to visualize what a 233 is going to print.

###Naming and access levels:

Snake is doing everything, so it is a SnakeGame class in reality.

There are several methods in the class that are only called internally. This means that they should be private methods instead.

###More spacing can help readability:

Crammed lines like this are harder to read:

for(x=1;x<width-1;x++)

Always putting a space after each arithmetical operator produces easier to read code, for the same reasons we put spaces between words.

for(x = 1; x < width-1; x++)

That's better.

I'm a "space man", so would add a couple more:

for (x = 1; x < width - 1; x++)