C++

Forum

 
Review my code =]

Jack Laughdale (6)
Hi im pretty new to c++ and I was hoping you guys could review this code I wrote.
its a game that makes the user guess a randomly generated number and then you have x amount of times to guess right. been using this to practice classes and keeping main() clean. Also I know the pointer is unnecessary, I put it there for practicing passing by reference.
heres the header
rand.h
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
 
// Class declarations
#ifndef RAND_H
#define RAND_H
#include <string>
class game
{
	

public:
	int randGen(int seed, int maxNum, int choice);
	int randGame(int setting, int num);		// if returns 1 call win() if 2 call lose()
	int  challengeSet(int *hardness);	// returns chalenge level to main
	void win();
	void lose();

};

#endif


and the
randGame.cpp (with all the functions)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
 
#include <cstdlib>
#include <ctime>
#include <iostream>
#include "rand.h"

using namespace std;

int game::challengeSet(int *hardness)
{
	cout << endl << "Please make choice: " << endl;					//setting the difficulty
		cin >> *hardness;	
	cout << endl << "Are you sure? Please confirm choice: " << endl;					//setting the difficulty
		cin >> *hardness;		
	switch (*hardness) 
	{
	case 1:
		cout << "LOL TOO EASY... OKAY";
			return 1;
			break;
	case 2: 
		cout << endl << "There we go... COME PREPARED!!" << endl;
		return 2;
		break;
	case 3:
		cout << endl << "This is suicide!!!" << endl;
		return 3;
	default:
		cout << endl << "Ivalid choice.... RESTARTING." << endl;
		 int wrong = 0;
		challengeSet(&wrong);
		break;
	}
}

int game::randGen(int seed, int maxNum, int choice)
{
	int randnum;																// random generation algorithem
	
	if (choice == 1)
	{
	srand(seed);
	randnum = 1+(rand()%maxNum);
	}
	else 
	{
		srand(time(0));
	randnum = 1+(rand()%maxNum);
	}

	return randnum;

}


void game::win()
{
	cout << endl << "HOORRRAYYYY YOU WIN, heres some cake!" << endl;			// called if win
}
void game::lose()
{
	cout << endl << "Sorry, you suck at this!!!!" << endl << "GAME OVER!!!" << endl;					// called if lose
}

int game::randGame(int setting, int num)
{
	cout << "Generating random number..." << endl;
	
																				// main game
	int guess;
	guess = 0;
	int numOfTries;
	switch (setting)
	{
	case 1: 
		numOfTries = 10;
		break;
	case 2: 
		numOfTries = 6;
		break;
	case 3: 
		numOfTries = 4;
		break;
	}

	cout << "You have " << numOfTries <<" tries..." << endl;		

	for (int tries = 1; tries <= numOfTries; tries++)
	{
		cout << endl <<"Guess number " << tries << " GO!!!" << endl;
		cin >> guess;
		
		
		if (guess == num)
		{ 
			return 1;
			
		}
			
		else if (guess < num)
		{
			cout << "Keep guessing Higher!!" << endl;
		}
		else if (guess > num)
		{
			cout << "Woah!! Slow down, go lower" << endl;
		}
		if (tries > numOfTries)
		{
			return 2;
			
		}
	}
	
	
}



and randMAin.cpp

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
 
#include <iostream>
#include <string>
#include "rand.h"
#include "randGen.h"

using namespace std;

int difficulty;	// for difficulty setting
int finalDiff;
int winOrLose;
int main()
{
	int cont;
	int seedNum = 0;
	int maxNumber = 99;
	int choice;
	
	int rNumber;
	game randG;
	


	cout << "You will guess a number from 1-???" << endl;
	cout << "Press any key to begin.........";
	 cin >> cont;
	
	cout << "Firstly would you like to seed a number, or just use the regular algoritham?" <<			// choice to seed a number
		    endl << "press 1 or 2 respectively" << endl;
	 cin >> choice;
	if(choice == 1)
	{
		cout << "Enter the seed number you would like to use: ";										// seed num
		 cin >> seedNum;
	}
	do
	{
	   cout << endl << "Now enter the maximum number you want to guess to (minimum is 100): ";				// max number to generate
		cin >> maxNumber;
	}
	while (maxNumber < 100);

	rNumber = randG.randGen(seedNum, maxNumber, choice);

	cout << endl << "Please make a choice" << endl <<
					"1. For Easy." << endl <<
					"2. For Medium" << endl << 
					"3. For Hard.";
	
	finalDiff = randG.challengeSet(&difficulty);

	winOrLose = randG.randGame(finalDiff, rNumber);
	
	

	if (winOrLose == 1)
	{
		randG.win();
	}
	else
	{
		randG.lose();
	}
	
	system("pause");
	return 0;

}

Jack Laughdale (6)
also ignore the #include "randGen.h" in randMain.cpp
thats old and Im taking it out XP
closed account (3CXz8vqX)
Your commenting....ish awful! ;) Write a comment about what each file does at the top of each file.

Try and summarise any blocks of code as well while you're at it. Perhaps put a comment next to the includes to explain what they do (the none standard ones I mean). Also explain your variables, what do they do?

Can you put your variables any nearer to where they are going to be used?

Line 28 RandMain.cpp use \n instead of endl, use endl to flush once you have finished with the text.

Line 29 shouldn't cin >> choice be one column to the left.

booradley60 (896)
I assume you're not worried about pedantry when creating a thread like this. I'll take a few passes at it and point out some things.

First, consistency. Ideally, the header file would be named the same as your implementation file (with the only difference being the file extension, naturally). Also, it is a good habit to name the header and implementation after the class whose code it contains.

Second, naming. This isn't FORTRAN, you have plenty of room to give your classes, functions, and variables descriptive names. Instead of class game, say something like class GuessingGame. Rename your .h and .cpp accordingly.

Third, whitespace. Try to use spaces instead of tabs. Not everyone's tab widths are the same and it can make for some funky indentation if you move the code around, especially if you're mixing in spaces.

Fourth, stay away from system("pause");. There is an entire thread dedicated to this stickied to the first page of the Beginners forum.

Fifth, try to avoid using namespace std; I think it would be better if you called out using std::cout; using std::cin; and using std::endl; There may be name collisions when you pull in the entire namespace (be especially careful with the word rand).
Last edited on
Jack Laughdale (6)
Lol I know the commenting is pretty bad Ravenshade, I'm still working on good commenting style.
Thanks booradley, I'll edit my code a bit, and get into these habits for future programs.
MiiNiPaa (8886)
About whitespaced: there is good rule "Ident with tabs, align with spaces". That means that you will press tab several time at the beginning of line depending on identation level and will not use them again till the end of the line. e.g:
Using tabs to ident and spaces to align here. Tabs in this forum are 8-spaces by default
1
2
3
4
 
{
	std::cout << "Hello " <<
	             "world";
}

(Using spaces to emulate 4-space tabs) Note how code stays aligned after switching to 4-space tabs. 
1
2
3
4
 
{
    std::cout << "Hello " <<
                 "world";
}

In next snipped I have used 8-space tabs (and some spaces) to align. Notice, how it is magled after switching to 4-space tabs:
1
2
3
4
 
{
    std::cout << "Hello " <<
             "world";
}


Ident style isn't consistent too. Choose one ( en.wikipedia.org/wiki/Indent_style ) and stick to it (I suggest K&R).

Try to stay away from global variables. I don't see, why you used it here.
Last edited on
Catfish3 (666)
(I suggest K&R).

And I suggest Allman, since that's what you're currently using. Also you forgot http:// MiiNiPaa.

http://en.wikipedia.org/wiki/Indent_style
ne555 (10692)
You shouldn't `srand()' at each number generation.
Just once in the whole program should be enough.
Another thing is that all generators will share the seed.

You can use your object state.

In `challengeSet()', ¿what would be the returned value if you put an invalid choice?
should loop instead of recurse there.
Topic archived. No new replies allowed.