C++

Forum

 
is this an example of good coding? need advice

Sellias13 (14)
Here is my code, I am new at coding and would like to know if there are better ways to do what i am currently doing.

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
 
void play(){
	cout<<"=========================="<<endl;
	cout<<"==  RED BLUE CARD GAME  =="<<endl;
	cout<<"==     By               =="<<endl;
	cout<<"==      Stephen Ellias  =="<<endl;
	cout<<"=========================="<<endl;
	cout<<""<<endl;
	cout<<""<<endl;
	while(z==1) // should be while(z==1)
	{
		cout<<""<<endl;
		r = rand() % 6 + 1;
		cout<<"Your total is "<< t <<"!"<<endl;
		cout<<"Pick a deck"<<endl;
		cout<<"Red or Blue"<<endl;
		cin>> x;
		cout<<""<<endl;
		placebet:
		cout<<"Place Your Bet"<<endl;
		cin>> e;
		if (e < 101){										//If Player Gives a Legal Bet
			if (x == "Red" || x == "red"){
				if (r == 1 || r == 2){							//Player has won bet
					t = t + e;										//Adding Bet to players total
					system("cls");									//Clearing the screen
					cout<<"=========================="<<endl;
					cout<<"==  RED BLUE CARD GAME  =="<<endl;
					cout<<"==     By               =="<<endl;
					cout<<"==      Stephen Ellias  =="<<endl;
					cout<<"=========================="<<endl;
					cout<<""<<endl;
					cout<<"You have won $"<< e <<endl;				//Telling player how much they have won
				}
					if (r == 3 || r == 4 || r == 5 || r == 6){	//Player Has Lost Bet
					t = t - e;										//Subtracting bet from players total
					system("cls");									//Clearing the screen
					cout<<"=========================="<<endl;
					cout<<"==  RED BLUE CARD GAME  =="<<endl;
					cout<<"==     By               =="<<endl;
					cout<<"==      Stephen Ellias  =="<<endl;
					cout<<"=========================="<<endl;
					cout<<""<<endl;
					cout<<"You have lost $"<< e <<endl;				//Telling the PLayer how much they have lost
				}
			}
			if (x == "Blue" || x == "blue"){
				if (r == 1 || r == 2 || r == 3){				//Player has won bet
					t = t + (e/2);									//Adding bet to players total
					system("cls");									//Clearing the screen
					cout<<"=========================="<<endl;
					cout<<"==  RED BLUE CARD GAME  =="<<endl;
					cout<<"==     By               =="<<endl;
					cout<<"==      Stephen Ellias  =="<<endl;
					cout<<"=========================="<<endl;
					cout<<""<<endl;
					cout<<"You have won $"<< e/2 <<endl;			//Telling the player how much they have won
				}
				if (r == 4 || r == 5 || r == 6){				//Player has lost bet
					t = t - (e/2);									//Subtracting bet from players total
					system("cls");									//Clearing the screen
					cout<<"=========================="<<endl;
					cout<<"==  RED BLUE CARD GAME  =="<<endl;
					cout<<"==     By               =="<<endl;
					cout<<"==      Stephen Ellias  =="<<endl;
					cout<<"=========================="<<endl;
					cout<<""<<endl;
					cout<<"You have lost $"<< e/2 <<endl;			//Telling the player how much they have lost
				}
			}
		}
		if (e > 101){											//If Player Gives Illegal Bet
			system("cls");											//Clear The Screen
			cout<<"=========================="<<endl;
			cout<<"==  RED BLUE CARD GAME  =="<<endl;
			cout<<"==     By               =="<<endl;
			cout<<"==      Stephen Ellias  =="<<endl;
			cout<<"=========================="<<endl;
			cout<<""<<endl;
			cout<<"You have placed an illegal bet"<<endl;			//Tell the player their bet is illegal
			cout<<"Place another bet that is under $100"<<endl;		//Asking the player for a bet under 100
			system("pause");										//Stopping to let the player read the rules
			system("cls");
			cout<<"=========================="<<endl;
			cout<<"==  RED BLUE CARD GAME  =="<<endl;
			cout<<"==     By               =="<<endl;
			cout<<"==      Stephen Ellias  =="<<endl;
			cout<<"=========================="<<endl;
			cout<<""<<endl;
			goto placebet;											//Going back to right before the player bets
		}
	}	
	}
vlad61 (212)
title should be its own seperate function - its not very important yet sticks out like a sore thumb
James Burnby (24)
The first thing to note is there is always a better way to do something. I have never gone back to software and found it perfect even after tweaking it for years.

As for best practices:
1. You repeat a lot of code especially in regards to couts. Try to put your CARD GAME couts in to one method. You can use a parameter for the RED or BLUE CARD GAME.

2. Placebet can be a separate method. That way all you need to do in your main code is:
1
2
3
 
while(z==1){ // or while(true)
   placeBest();
}

- GOTO can always be avoided, it makes code harder to read and can often end up in infinite loops or random crashes.

3. Rather than testing for both Red and Blue you could convert the inputted text to uppercase and check for RED or BLUE. Or just check the first uppercase character e.g. if(x[0]=='R') for red

4. After your if e < 101 condition you could use else if(e > 101) instead of just if e > 101.

Or you could call continue at the end of the e < 101 block. That way you do not need the condition e > 101.

5. Another best practice is to use as many constant values as possible rather than straight typed text. e.g. #define REDINPUT "RED"


Remember a customer doesn't care how your code is written though. If it works and works well, then its fine.
Topic archived. No new replies allowed.