Problem with functions, loops, etc.

Pages: 12
Jan 1, 2012 at 9:39am
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;
}
Jan 1, 2012 at 9:47am
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 Jan 1, 2012 at 10:08am
Jan 1, 2012 at 10:08am
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 Jan 1, 2012 at 10:15am
Jan 1, 2012 at 10:12am
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.
Jan 1, 2012 at 10:23am
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 Jan 1, 2012 at 10:25am
Jan 1, 2012 at 1:32pm
Great post stewbond :D
Jan 1, 2012 at 3:55pm
Did you just write the program for him?
Jan 1, 2012 at 4:05pm
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.
Jan 1, 2012 at 9:20pm
Not what it looks like to me...
It's just, it doesn't help people learn when they're given an answer.
Jan 2, 2012 at 8:46pm
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