Smaller if else code?

I'm in the process of teaching myself C++ with the book Accelerated C++, on chapter 3. But I was wondering if I could shorten this if else code. Is there a simpler way to do that? I wrote this up just now after having the idea at work. I don't really like all of the if's and else's.

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
#include <iostream>
#include <conio.h>

// say what standard-library names we use
using std::cin;
using std::cout;
using std::endl;
using std::string;

int main()
{	
	cout << "Shoot a target between 1 and 5!" << endl;
	int target;
	cin >> target; 

	if  (target == 1)
		cout << "Head shot!" << endl;
	else
	
	if  (target == 2) 
		cout << "Body shot!" << endl;
	else
	
	if	(target == 3)
		cout << "Arm shot!" << endl;
	else
	
	if	(target == 4)
		cout << "Knee shot!" << endl;
	else
	
	if	(target == 5)
		cout << "Nut shot!" << endl;
	else
	
	if (target != 1,2,3,4,5)
		cout << "You missed!" << endl;

	_getch();
	return 0;
}
Last edited on
if (target != 1,2,3,4,5) You don't really need this if because you know that target can't be any of 1, 2, 3, 4 or 5 because you have checked that above.

else and if after each other is often written on the same row.

1
2
3
4
5
6
7
8
9
10
11
12
if (target == 1)
	cout << "Head shot!" << endl;
else if (target == 2) 
	cout << "Body shot!" << endl;
else if (target == 3)
	cout << "Arm shot!" << endl;
else if (target == 4)
	cout << "Knee shot!" << endl;
else if (target == 5)
	cout << "Nut shot!" << endl;
else
	cout << "You missed!" << endl;


You can also use a switch statement if you like.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
switch (target)
{
case 1:
	cout << "Head shot!" << endl;
	break;
case 2:
	cout << "Body shot!" << endl;
	break;
case 3:
	cout << "Arm shot!" << endl;
	break;
case 4:
	cout << "Knee shot!" << endl;
	break;
case 5:
	cout << "Nut shot!" << endl;
	break;
default:
	cout << "You missed!" << endl;
}


Or if you want you can make use of an array.
1
2
3
4
5
string targetTexts[] = {"Head shot!", "Body shot!", "Arm shot!", "Knee shot!", "Nut shot!"};
if (target >= 1 && target <= 5)
	cout << targetTexts[target - 1] << endl;
else
	cout << "You missed!" << endl;
Last edited on
Another variation

1
2
3
4
5
6
7
8
9
10
11
12
13
    const char* pchMsg = "You missed!";

    switch (target)
    {
        case 1: pchMsg = "Head shot!"; break;
        case 2: pchMsg = "Body shot!"; break;
        case 3: pchMsg = "Arm shot!" ; break;
        case 4: pchMsg = "Knee shot!"; break;
        case 5: pchMsg = "Nut shot!" ; break;
        default: {/* leave default */}
    }

    cout << pchMsg << endl;


And

if (target != 1,2,3,4,5)

isn't checking that target doesn't equal 1, 2, 3, 4, or 5

It's checking that target isn't one and then applying the comma operator (aka sequence operator) to evaluate your other "expressions", which being just numbers don't do enything.
http://www.cplusplus.com/doc/tutorial/operators/

To test that target is not 1, 2, 3, 4, or 5 you need

if (target != 1 && target != 2 && target != 3 && target != 4 && target != 5)

Peter87's if, else sequence is much better!
Last edited on
if (target != 1,2,3,4,5)
to make it simple, comma operator are evaluated from left to right
(http://www.cplusplus.com/doc/tutorial/operators/)
"if" statement will evaluate last condition, in this example is 5.
in c++, any integer value except 0 considered true.
cmiiw
You can use an array to do this.
1
2
3
4
5
6
7
8

const char* str[] = {"Head Shot!", "Body Shot!", "Arm Shot!", "Knee Shot!", "Nut Shot!"};

unsigned int input;
cin >> input;

cout << ((input-1 > 4)? "You missed!" : str[i-1] );
Last edited on
For example you could rewrite your program the following way

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
#include <iostream>
#include <conio.h>

// say what standard-library names we use
using std::cin;
using std::cout;
using std::endl;

int main()
{
	const char *text[] = { "You missed!", "Head shot!",
		                   "Body shot!", "Arm shot!",
		                   "Knee shot!",  "Nut shot!" };

	cout << "Shoot a target between 1 and 5!" << endl;

	unsigned int target;
	cin >> target; 

	if ( target < 1 || target > 5 ) cout << text[0];
	else cout << text[target];
	cout << endl;

	_getch();
	return 0;
}
Thanks everyone. i forgot about the && operator. I won't make that same problem with the last if else again. But I like peter's array solution. I haven't learned about switches or arrays yet but it beats the way i was doing this. Turns out I didn't learn about comma operators yet either so that explains my mistake lol.
Last edited on
Topic archived. No new replies allowed.