Problem with functions, loops, etc.

Pages: 12
Wow, messy thread.

Here is what I'd do for the choices() function. It's much shorter and easier to read. Comments highlight changes.

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
int choices ()
{
    int choice; // This doesn't need to be passed into the argument.  Make it local.
    char letter; // This is best as a char
    cout << "Would you like to:\nA. Punch\nB. Kick\nC. Slash\nD. Use Magic\nE. Use A Potion\nF. Commit Suicide\nPlease enter the letter of your choice:";
    // A=1 B=2 C=3 D=4 E=5 F=6

	bool valid_input = false; // loop until the input is valid.
	while (!valid_input)
	{
		cin >> letter;

		switch (letter) // Implement a switch statement instead of else-if
		{
		case 'A':
		case 'a':
			choice = 1;    valid_input = true;    break;
		case 'B':
		case 'b':
			choice = 2;    valid_input = true;    break;
		case 'C':
		case 'c':
			choice = 3;    valid_input = true;    break;
		case 'D':
		case 'd':
			choice = 4;    valid_input = true;    break;
		case 'E':
		case 'e':
			choice = 5;    valid_input = true;    break;
		case 'F':
		case 'f':
			choice = 6;    valid_input = true;    break;
		default:
			cout << "You did not enter a valid choice. Please try again: ";
			break;
		}
	}

    return choice;
}
Regarding damage1():
Functions cannot return two values. Actually you don't really need to return choice at all and you don't need to pass damage. I'd replace that function with:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
int damage1(int choice) // Damage does not need to be input here
{
    //move srand to the main, if we call it multiple times we can get some non-random behavior

    switch(choice)
    {
    case 1:
        return rand() % 10 + 5; //returning the damage done only
    case 2:
        return rand() % 15 + 5; // You could set a variable int damage (declared locally)
    case 3:                     // so damage = rand() % 30 + 1;
        return rand() % 30 + 1; // then return only damage at the end.
    case 4: 
        return rand() % 75 + -50;
    case 5: 
        return (rand() % 100 + 25) * -1; // Times -1 because it is healing, not damage.
    case 6:
        return rand() % 100 + 50;
    default:
        return 0;    //protection case if choice is invalid.
    }
}


the computer() function isn't bad, but damage does not need to be inserted into the function because it is not used. Make damage a local variable.

Also, there is the == vs = that people have mentioned.

Finally, it would be nice to see this as a general function that can be used for any player (computer or you). For that reason, lets change the "You kicked" to a more variable solution. I'd do the following (I'm also trying to keep your code in tact so you can see the changes better).

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
int computer(int choice, string name)
{
	int damage = 0; //Make this local
    if (choice == 1)
    {
        damage = damage1 (choice);
        cout << name << " punched the enemy for " << damage << " healh" << endl; //Add the name at the start so this function can be used for both sides
    }
    else if (choice == 2)
    {
        damage = damage1 (choice);
        cout << name << " kicked the enemy for " << damage << " healh" << endl; // Add endl at the end.
    }
    else if (choice == 3)
    {
        damage = damage1 (choice);
        cout << name << " slashed the enemy for " << damage << " healh" << endl;
    }
    else if (choice == 4)
    {
        damage = damage1 (choice);
        cout << name << " used magic on the enemy for " << damage << " healh" << endl;
    }
    else if (choice == 5)
    {
        damage = damage1 (choice);
        cout << name << " used a potion which restored " << damage << " healh" << endl;
    }
    else if (choice == 6)
    {
		damage = 9999; // Set damage or nothing will happen
        cout << name << " commited suicide. Now that wasn't very smart" << endl;
    }
    return damage;
}
Last edited on
Finally the main()

Now we can put it all togeather. Don't forget to use a loop so that things continue until someone wins.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
int main()
{
    int yhealth = 999, ehealth = 999, choice;
	srand(time(NULL));

    cout << "Battle: Begin!" << endl;

	while (yhealth > 0 && ehealth > 0)
	{
		displayhealth(yhealth, ehealth);

		//Your move
		choice = choices();
		ehealth -= computer(choice, "You"); //Subtract damage from computers health

		//Computer move
		choice = (rand() % 6) + 1; // Let the computer make a hit.
		yhealth -= computer(choice, "Computer"); //I also put the person dealing damage here as an input.
	}
    return 0;
}


Stick all of this together and you've got something that nearly works. I tried it out and found that the suicide and potion conditions are still not great because damage should be done to yourself and instead you do that damage to the other person.
Last edited on
Your code is poorly readable.

Try optimizing it better into less functions, and structure enemy and character data into sets for more ease of access per data.
There, I've changed main() so that it deals damage to the correct target. I've stuck it all together and it works for me. Cheers

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
int main()
{
    int yhealth = 999, ehealth = 999, choice, damage;
    srand(time(NULL)); // The random seed is done in the main once and only once.

    cout << "Battle: Begin!" << endl;

    while (yhealth > 0 && ehealth > 0)
    {
        displayhealth(yhealth, ehealth);

        //Your move
        choice = choices();
        damage = computer(choice, "You");

        if (choice < 5)         //Doing damage to the enemy 
            ehealth -= damage;  //Subtract damage from computers health
        else                    //Doing damage to yourself
            yhealth -= damage;  //Subtract damage from your health

        //Computer move
        choice = (rand() % 5) + 1; // Let the computer make a hit.
        damage = computer(choice, "Computer");

        if (choice < 5)
            yhealth -= damage;
        else
            ehealth -= damage;
    }
    return 0;
}
Last edited on
Great post stewbond :D
Did you just write the program for him?
I made corrections to his functions and highlighted differences in comments. To be any fun, this would only be a small function in a program and would need some modifying.
Not what it looks like to me...
It's just, it doesn't help people learn when they're given an answer.
Guess I still need a little more learning to do! I still don't entirely understand switch statements and that is why I didn't use them. Sorry for the inconvenience. I will try to put all your suggestions into my code. Thanks for the help!
Topic archived. No new replies allowed.
Pages: 12