HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Snake game with a sound system

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withsystemsnakegamesound

Problem

Is there any way to improve it?

```
#include
#include
#include"MMSystem.h"
#include
#include
#include
#include
#include

using namespace std;

/**/

/***/
struct player
{
string name;
int score;
};
/**/
const int mx1=1,mx2=36,my1=1,my2=20;
int D=3;
int x=(mx2+mx1)/2;
int y=(my2+my1)/2;
bool running=true;
player current;
const int Total_Players=10;
unsigned int z=0;
player high[Total_Players+1];
char MUSIC[100];

/**/
/****/
void score_print()
{
system("CLS");
running=true;
gotoxy(30,1);
cout**/
void update_highscore()
{
fstream myfile;
getline(cin,current.name);
current.score=food;
while(current.name.length()=high[i].score)
{
for(int j=Total_Players-1;j>=i;j--)
{
high[j+1] = high[j];
}
high[i] = current;
break;
}
}
myfile.open("bin\\score.txt");
if(myfile.is_open())
{
for(int i=0;i0;i--)
{
X1[i]=X1[i-1];
Y1[i]=Y1[i-1];
}
Food(X[0],Y[0]);
if(X[0]==mx1||X[0]==mx2||Y[0]==my1||Y[0]==my2||k==10)
{
running=false;
i=600;
while(i1)
D--;
else if(L==13)
{
D=800-D*75;
break;
}
system("CLS");
for(i=0;i*/
void score()
{
fstream myfile;
string X[Total_Players];
myfile.open("bin\\score.txt");
if(myfile.is_open())
{
while(!myfile.eof())
{

for(int i=0;i>high[i].score;

}
}
myfile.close();
}
for(int i=0;i**/
/****/
void sound_play()
{
/****/
PlaySound(MUSIC, NULL, SND_ASYNC|SND_FILENAME|SND_LOOP);
}

/****/
void music_select()
{
int L=0

Solution

There are many improvements that could be applied to this code. As it stands, it has several issues or "code smells":

-
Use of global data: You are using C++, which supports Object-Oriented programming, and you also already make use of classes in your program, so why declare a bunch of global data when all that could very easily be properly scoped into classes or functions?

-
Very weak names for global variables: To further aggravate the issue of the globals, some are declared with ludicrous names, such as x, y and D. Those will not only clash with other names you might want to declare, but also convey no meaning or information. The more visible is the scope of a variable, the better and more descriptive its name should be. Loop counter variables are commonly named i, j and k because they have tiny scopes (and for historical reasons, also).

-
Magic numbers: Your code is swamped with magic numbers, which makes it very hard to understand and create a mental model of the thing.

-
Using namespace: Very questionable practice to say the least. Read more about it here.

-
Use of goto: In main(), where a simple for(;;) or while would do. That goto also threads over the declaration of GAME, which is a usage style full or caveats and not well known by most programmers. A 'traditional' loop would be much clearer. There are other instances too that seem to exist because you've failed to better divide the code into functions.

-
Lack of spacing makes it very hard to read: Compare this:

if(X[0]==mx1||X[0]==mx2||Y[0]==my1||Y[0]==my2||k==10)


To a more spaced equivalent:

if (X[0] == mx1 || X[0] == mx2 || Y[0] == my1 || Y[0] == my2 || k == 10)


My opinion, but I think the issues mentioned above makes it hard to review the code in more detail. So my suggestion would be that you first fix those issues and then post a followup question to get more reviews. Bet of luck to you.

Code Snippets

if(X[0]==mx1||X[0]==mx2||Y[0]==my1||Y[0]==my2||k==10)
if (X[0] == mx1 || X[0] == mx2 || Y[0] == my1 || Y[0] == my2 || k == 10)

Context

StackExchange Code Review Q#93730, answer score: 3

Revisions (0)

No revisions yet.