patterncppMinor
Snake game with a sound system
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
```
#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
-
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
-
Lack of spacing makes it very hard to read: Compare this:
To a more spaced equivalent:
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.
-
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.