Standard Programming

Hi, I just built my first game. I've had no teacher, I'm just learning by myself. I know there are certain ways to do some things.. standardized programming. Is there anything I need to improve on here?

It's a kind of dungeon crawl game, where the user has to reach the treasure 'X', without hitting a trap 'T'. And the traps keep multiplying.

 ``123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166`` ``````#include using namespace std; //GLOBAL VARIABLES char board[8][10]; int x, y; int m = 0, n = 0; int trapx, trapy; int win = 0, lose = 0; //FUNCTIONS void buildboard(); void initialize(); void moveplayer(char userinput); int checkwin(int n, int m); void settrap(); int checklose(int n, int m); int main() { //INITIALIZE char userinput; initialize(); settrap(); //DO WHILE NO WINNER OR LOSER while(win==0 && lose==0){ buildboard(); settrap(); //ONLY CONTINUES IF VALID INPUT while(userinput != 'w' && userinput != 's' && userinput != 'a' && userinput != 'd') { cout << "\t\nPlayer move up, down, left, right (w,s,a,d)?"; cin >> userinput; } moveplayer(userinput); userinput = '.'; //RESET } //FINALLY OUTPUT RESULT buildboard(); if (win == 1){ cout << "\n\n\t***WINNER!!!***\n"; } else { cout << "\n\n\t***YOU LOSE!!!***\n"; } } void initialize() { //GIVES ALL SPACES NULL VALUE for(int i=0; i<8; i++) { for(int j=0; j<10; j++) { board[i][j] = '.'; } } //TREASURE RANDOM while(x+y == 0){ srand(time(0)); x = rand() % 10; y = rand() % 8; board[y][x] = 'X'; } //PLAYER AT 0,0. board[m][n] = 'G'; } void buildboard() { system ("cls"); cout << "\n\tDUNGEON CRAWL \n"; cout << "\n\tHurry they're multiplying... \n\n"; for(int i=0; i<8; i++) { cout << "\t "; for(int j=0; j<10; j++) { cout << board[i][j]; } cout << endl; } } void moveplayer(char userinput) { //W UP, S DOWN, A LEFT, D RIGHT; //DOES NOT LET PLAYER MOVE OFF BOARD board[n][m] = '.'; if(userinput =='w' && n!=0){ n = n - 1; } else if(userinput =='s' && n!=7){ n = n + 1; } else if(userinput =='a' && m!=0){ m = m - 1; } else if(userinput =='d' && m!=9){ m = m + 1; } //before reassigning G check for treasure or trap!!****** //if win win = checkwin(n,m); //if lose lose = checklose(n,m); //else board[n][m] ='G'; } void settrap() { //TRAP RANDOM bool go = false; while(go == false){ trapx = rand() % 10; trapy = rand() % 8; if(board[trapy][trapx]=='.'){ board[trapy][trapx] = 'T'; go=true; } } } int checkwin(int n, int m) { //IF PLAYER G FINDS TREASURE X if(board[n][m] == 'X'){ return 1; buildboard(); cout << "\n\n\t***WINNER!!!***\n"; } else { return 0; } } int checklose(int n, int m) { //IF PLAYER G FINDS TRAP if(board[n][m] == 'T'){ return 1; buildboard(); cout << "LOSER"; } else { return 0; } } ``````
Hi,

First of all your code is a pretty good effort for someone who is self taught - so well done!

Here are some tips:

Line 3:

Try not to bring in the whole std namespace into the global namespace. It can cause problems - there are a number of functions in std that might conflict with your variables - such as std::left and `std::right` plus many more for example. If you have a look at the code from advanced members of this forum you will see they prefix everything from std with `std::` as in `std::cout`. You can do `using std::cout;` and similar for things that are used frequently if you want. Technically you should put your own stuff in your own namespace - read for how to do this.

Lines 6 - 10:
Really avoid using global variables - there can be confusion between the global & local variables. They generally lead to messy code.

Line 36:
I personally hate dislike :) constructs like this. IMO they are non scalable & messy. A much better option is to use a switch with a quit option, it might be more code - but it is much clearer & maintainable IMO. Sometimes it is worth it to have longer code instead of a nasty one liner.

bool variables lines 29 - 48:

I prefer to use these with true / false rather than 0 or 1. Technically no difference, just easier IMO. Although you have used them later on in your program.

`i` or `j` variables:

This is really nit picking, but I don't like these either!! I have seen before where someone had a mass of `i` and `j` variables for matrix processing & there was an error. So we did a find/replace changing them to `row `& `col` and the error was obvious straight away !

`system `calls

These are not preferred because they are not portable & pose a security threat.

If you format your IDE to use 4 spaces instead of tabs, the indenting might display better on this web page, as it always uses 8 spaces for tabs, whereas 4 spaces is 4 spaces.

Finally, there are heaps of resources in the reference section at the top left of this page + articles & tutorial (Wasn't sure if you were aware of these)

Hope all goes well :+)
Thanks a lot TheIdeasMan, I didn't know half of this stuff, I'll get to work. :D
Do you think the board[][] should not be a global variable even though it's used in every function?
lorilew:
 Do you think the board[][] should not be a global variable even though it's used in every function?

This is an interesting question, because it depends on what you intend to do. For something of this size, having board as a global doesn't really hurt anything. However, if you intend to increase the size of your game and have multiple levels, then having board has a global can complicate things.

That's a pretty good program, especially for someone who is new and self taught. It seems to me that looking at your code, you haven't learned about classes or pointers/references yet. Those would really help you with your program.

It's futile to explain how learning about those things would help you with your program. (There are many others who can tell you much more than I can and in a better way. Learning those topics will make it apparent where you can improve your program.)

It's still a good program though! I remember when I wrote something like that when I was starting out. We all start from somewhere, and grow into something awesome.
This is the new version. I couldn't figure out how to verify the users input, but I'm still trying to interpret some of these articles :) And I go rid of system "cls" but it seems a little messy now, but it's ok.

 ``123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175`` ``````#include #include //GLOBAL VARIABLES char board[8][10]; //FUNCTIONS void buildboard(); void initialize(); int moveplayer(char userinput); bool checkwin(int row, int col); void settrap(); bool checklose(int row, int col); void ClearScreen(); int main() { //INITIALIZE char userinput; initialize(); settrap(); bool stop = false; //DO WHILE NO WINNER OR LOSER while(stop == false){ buildboard(); settrap(); std::cout << "\n Player move up, down, left, right (w,s,a,d)?\n\n\n\n"; std:: cout << " "; std::cin >> userinput; stop = moveplayer(userinput); userinput = '.'; //RESET } } void initialize() { int col = 0,row = 0; //GIVES ALL SPACES NULL VALUE for(int i=0; i<8; i++) { for(int j=0; j<10; j++) { board[i][j] = '.'; } } //TREASURE RANDOM while(col+row == 0){ srand(time(0)); col = rand() % 10; row = rand() % 8; board[row][col] = 'X'; } //PLAYER AT 0,0. board[0][0] = 'G'; } void buildboard() { ClearScreen(); std::cout << "\n DUNGEON CRAWL \n"; std::cout << "\n Hurry they're multiplying... \n\n"; for(int row=0; row<8; row++) { std::cout << "\t "; for(int col=0; col<10; col++) { std::cout << board[row][col]; } std::cout << "\n"; } } int moveplayer(char userinput) { int col,row; //need to set m,n to player's position for(int i=0; i<10; i++){ for(int j=0; j<8; j++){ if(board[j][i] == 'G'){ col = i; row = j; break; } } } //W UP, S DOWN, A LEFT, D RIGHT; //DOES NOT LET PLAYER MOVE OFF BOARD board[row][col] = '.'; if(userinput =='w' && row!=0){ row--; } else if(userinput =='s' && row!=7){ row++; } else if(userinput =='a' && col!=0){ col--; } else if(userinput =='d' && col!=9){ col++; } //before reassigning G check for treasure or trap!!****** //if win if (checkwin(row,col) == true){ std::cout << "\n WINNER!\n\n\n"; return true; } //if lose else if(checklose(row,col) == true){ std::cout << "\n Sorry you LOSE!\n\n\n"; return true; } else{ board[row][col] ='G'; return false; } } void settrap() { //TRAP RANDOM int col, row; bool ok = false; while(ok == false){ col = rand() % 10; row = rand() % 8; if(board[row][col]=='.'){ board[row][col] = 'T'; ok=true; } } } bool checkwin(int row, int col) { //IF PLAYER G FINDS TREASURE X if(board[row][col] == 'X'){ return true; buildboard(); } else { return false; } } bool checklose(int row, int col) { //IF PLAYER G FINDS TRAP if(board[row][col] == 'T'){ return true; buildboard(); } else { return false; } } void ClearScreen() { std::cout << std::string( 100, '\n' ); } ``````
closed account (j3Rz8vqX)
Hey, I hope you don't mind.

I've decided to complicate the code, as well as simply:

Edited-Exported Code: Based on the original post: http://pastie.org/8684192#11,18,30,48,51,117,135,143-148

Hopefully the above helps motivate new ideas.

Nice program, good job.

Edit: The above should not have impacted your game... hopefully not =D
Last edited on
As Dput has alluded, the conditional statements always evaluate to true or false, so this:

`while(stop == false){`

can be simplified to:

`while(!stop){`

You have a boolean variable here, so that is easy to understand. It also works for integers - when it's 0 it's false. This can be useful when one has a loop that decrements - so it ends when the variable gets to 0.

`while(userinput != 'w' && userinput != 's' && userinput != 'a' && userinput != 'd')`

You may not have got around to changing it yet - but it would be better using a switch. It's important to do error checking on user input, so we can't leave that out. Here is a switch inside a while:

http://www.cplusplus.com/forum/beginner/92070/#msg499037

Here's a slightly different version with some of the game logic in:

 ``12345678910111213141516171819202122232425262728293031`` ``````bool Quit = false; while (!Quit) { // get user input switch (userinput) { case 'w' : if (row) { // row not zero row--; } else { //print error msg and let it loop again } break; case 's' : if (row!=7) { //print error msg and let it loop again } break; // etc for other options case 'q' : // user wants to quit Quit = true; break; default: // process errors // doing nothing here makes it loop again break; } }``````

One should use a `std::endl` at key points, because it flushes the buffer. Use `\n` as you have for the rest of the time.

@Dput

As I mentioned to the OP, I prefer Row & Col for variable names, rather than n & m, for the same reason I explained to the OP. Btw `n = n - 1` is more concisely written as `--n` or `n--`. The prefix version decrements the value before it is used.

Any way I hope you are all having good fun - I am, anyone would think it was the wet season at this end, so I have time to reply to you guys 8>D

EDIT:

I saw you earlier post about the problems 10's and 8's. The usual thing to do there is make them const variables, and just use the variable name from there on.

 ``12345678910`` ``````const int RowSize = 8; const int ColSize = 10; char board[RowSize ][ColSize ]; for(int Row =0; Row < RowSize ; ++Row) { for(int Col=0; Col
Last edited on
Hey ThedIdeasMan, MaxterTheTurtle and Dput, thanks for all the detailed feedback, I've actually learnt a lot today!

This is it so far now:
http://pastie.org/8684687

I think I'm gonna have a look at learning classes next. And I just don't understand pointers... I'm hoping to just absorb them unconsciously.
Last edited on
Hi lorilew,

Another thing for you to look at is the STL, there are containers that hold any type of object, & algorithms that manipulate containers. A `std::vector` is similar to an array, but it can resize itself & all the containers have properties (like size for example) that one can inquire about. Other containers worth looking at are `std::string` & `std::map`. Simple algorithms might include `std::sort` or `std::find`