C++

Forum

 
for loop not working properly

Pages: 12
lithony (26)
Write your question here.
the program is about two heroes fighting against each other and *operator displays the name of the winner and displays the number of rounds that took them to finish the battle.

However, the code displays the wrong number for "in # rounds".

For example,
when it should be winner is hercules in 4 rounds, it says 3 rounds
winner is atlanta in 4 rounds, it says 3 rounds
winner is hector in 7 rounds, it says 6 rounds.

Anyone who can find what is causing this problem?

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
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
 
  Main.cpp

    #include <iostream>
    #include "Hero.h"
    
    using namespace std;
    using namespace sict;
    
    int main() {
    
    	cout << "Greek Heroes";
    	Hero moneyhunger("", 40, 4);
    	Hero hercules("Hercules", 32, 4);
    	Hero theseus("Theseus", 14, 5);
    	Hero oddyseus("Odysseus", 15, 3);
    	Hero ajax("Ajax", 17, 5);
    	Hero achilles("Achilles", 20, 6);
    	Hero hector("Hector", 30, 5);
    	Hero atalanta("Atalanta", 10, 3);
    	Hero hippolyta("Hippolyta", 10, 2);
    
    	cout << endl << "Quarter Finals" << endl;
    	const Hero& greek_winner0 = moneyhunger * hector;
    	const Hero& greek_winner1 = achilles * hector;
    	const Hero& greek_winner2 = hercules * theseus;
    	const Hero& greek_winner3 = oddyseus * ajax;
    	const Hero& greek_winner4 = atalanta * hippolyta;
    
    	cout << endl << "Semi Finals" << endl;
    	const Hero& greek_winner_semifinal1 = greek_winner1  * greek_winner2;
    	const Hero& greek_winner_semifinal2 = greek_winner3  * greek_winner4;
    
    	cout << endl << "Finals" << endl;
    	greek_winner_semifinal1 * greek_winner_semifinal2;
    	system("pause");
    	return 0;
    }


Hero.cpp

    #include "Hero.h"
    #include <iostream>
    #include <cstring>
    using namespace std;
    namespace sict {
    
    	Hero::Hero()
    	{
    		m_name[0] = '\0';
    		m_health = 0;
    		m_attack = 0;
    
    	}
    
    
    	 
    	Hero::Hero(char name[], int health, int attack)
    	{
    
    		if (m_name != nullptr || m_name != "") {
    			strcpy(m_name, name);
    
    		}
    		else {
    			m_name[0] = '\0';
    		}
    
    		m_attack = attack;
    		m_health = health;
    
    
    	}
    
    	void Hero::operator-=(int attack) {
    		if (attack > 0) {
    			m_health -= attack;
    		}
    		if (attack > m_health) {
    			m_health = 0;
    		}
    	}
    	bool Hero::isAlive() const {
    		if (m_health > 0) {
    			return true;
    		}
    		else {
    			return false;
    		}
    	}
    	int Hero::attackStrength() const {
    		if (m_attack == 0) {
    			return 0;
    		}
    		else {
    			return m_attack;
    		}
    	}
    	ostream& operator<<(ostream& os, const Hero& hero) {
    		if (hero.m_name == '\0') {
    			os << "No Hero";
    		}
    		else {
    			os << hero.m_name;
    		}
    		return os;
    	}
    	const Hero& operator*(const Hero& first, const Hero& second) {
    		cout << "Ancient Battle! ";
    		cout << first;	
    		cout << " vs ";
    		cout << second;
    		cout << " : ";
    		Hero A = first;
    		Hero B = second;
    		const Hero *winner = nullptr;
    		int max_rounds = 0;
    		while (A.isAlive() && B.isAlive() && max_rounds < 200) {
    			max_rounds++;
    			A -= B.attackStrength();
    			B -= A.attackStrength();
    		
    		}
    
    		bool draw;
    
    		if (A.isAlive() && B.isAlive()) { draw = true; }
    		else { draw = false; }
    
    		if (draw) {
    			winner = &first;
    		}
    		else if (A.isAlive())
    		{
    			winner = &first;
    		}
    		else {
    			winner = &second;
    		}
    		cout << "Winner is ";
    		cout << *winner;
    		cout << " in " << max_rounds << " rounds. " << endl;
    		return *winner;
    
    	}
    }


Hero.h

    #ifndef SICT_HERO_H_
    #define SICT_HERO_H_
    #include <iostream>
    namespace sict {
    	class Hero {
    		char m_name[41];
    		int m_health;
    		int m_attack;
    	public:
    		Hero();
    		Hero(char name[], int health, int attack);
    		void operator-=(int attack);
    		bool isAlive() const;
    		int attackStrength() const;
    		friend std::ostream& operator<<(std::ostream& os, const Hero& hero);
    	};
    	const Hero& operator*(const Hero& first, const Hero& second);
    }
    #endif

jlb (4973)
Perhaps you need to convert from zero based counting to counting based on 1?

1
2
3
4
5
6
7
8
9
10
 
    	const Hero& operator*(const Hero& first, const Hero& second) {
 ...
    		int max_rounds = 0; // Perhaps this should be 1?
    		while (A.isAlive() && B.isAlive() && max_rounds < 200) {
    			max_rounds++;
    			A -= B.attackStrength();
    			B -= A.attackStrength();
    		
    		}
...



jonnin (11429)
it starts a 0, adds 1 and fights, checks for done, adds 1 and fights, checks for done...
so first round is round 1.
but after a winner is found, it does NOT increment again.

for example.

round 1 ... fight, max goes 0 to 1
round 2 ... fight max goes 1 to 2
round 3 ... fight, max goes 2 to 3 second is killed and first wins...

writes 3. This seems to be correct. ?

however I will observe that first has an advantage. Its coded so that if both are killed, first wins I think (?).
Last edited on
lithony (26)
@jlb if i start from 1, the correct ones go off by 1 as well.
lithony (26)
heres the output it should be:

in 4 rounds
in 4 rounds
in 3 rounds
in 4 rounds
in 7 rounds
in 2 rounds
in 4 rounds

but it displays like this:
in 4 rounds
in 3 rounds
in 3 rounds
in 3 rounds
in 6 rounds
in 2 rounds
in 3 rounds

so 2nd, 4th 5th and last one is off by 1
jlb (4973)
Is the winner different (first versus second versus draw)?

What happens if the Attack is equal to the opponents Health?

lithony (26)
no winner is same for both. If I set the if the attack = m_health, it messes up the number of rounds even more
jlb (4973)
It would be helpful if you posted the actual output of your program not a condensed version. Here is what the program produced for me:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
Greek Heroes
Quarter Finals
Ancient Battle!  vs Hector : Winner is  in 7 rounds.                             
Ancient Battle! Achilles vs Hector : Winner is Hector in 4 rounds. 
Ancient Battle! Hercules vs Theseus : Winner is Hercules in 3 rounds. 
Ancient Battle! Odysseus vs Ajax : Winner is Ajax in 3 rounds. 
Ancient Battle! Atalanta vs Hippolyta : Winner is Atalanta in 3 rounds. 

Semi Finals
Ancient Battle! Hector vs Hercules : Winner is Hector in 6 rounds. 
Ancient Battle! Ajax vs Atalanta : Winner is Ajax in 2 rounds. 

Finals
Ancient Battle! Hector vs Ajax : Winner is Hector in 3 rounds.


cire (8284)
1
2
3
4
5
6
7
8
 
    void Hero::operator-=(int attack) {
        if (attack > 0) {
            m_health -= attack;
        }
        if (attack > m_health) {
            m_health = 0;
        }
    }


What is m_health when it starts at 6 and attack is 4 when this function returns?
lithony (26)
It should print out
Greek Heroes
Quarter Finals
Ancient Battle! vs Hector : Winner is in 7 rounds.
Ancient Battle! Achilles vs Hector : Winner is Hector in 4 rounds.
Ancient Battle! Hercules vs Theseus : Winner is Hercules in 4 rounds.
Ancient Battle! Odysseus vs Ajax : Winner is Ajax in 3 rounds.
Ancient Battle! Atalanta vs Hippolyta : Winner is Atalanta in 4 rounds.

Semi Finals
Ancient Battle! Hector vs Hercules : Winner is Hector in 7 rounds.
Ancient Battle! Ajax vs Atalanta : Winner is Ajax in 2 rounds.

Finals
Ancient Battle! Hector vs Ajax : Winner is Hector in 4 rounds.
lithony (26)
it should return 2 because 6-4 = 2
cire (8284)
It shouldn't return anything because the return type is void

But, yes, m_health probably should be 2 when the function returns, however if you were to step through that code you would see that it is not.
lithony (26)
Anyone who knows why this rounds decrement by 1 not in every case but it appears in some cases?
jlb (4973)
Yes, have you single stepped through your operator-= while watching the various class variable values?

If you don't know how to use your debugger then you need to put some print statements in several places in that function.

Here is a sample output with a couple print statements in that function showing some of the class variables at different times.
1
2
3
4
5
6
7
8
9
10
11
12
13
 
In operator -= before if() Hector health: 30 Damage: 5
In operator -= after if() Hector health: 25 Damage: 5
In operator -= before if() Ajax health: 17 Damage: 5
In operator -= after if() Ajax health: 12 Damage: 5
In operator -= before if() Hector health: 25 Damage: 5
In operator -= after if() Hector health: 20 Damage: 5
In operator -= before if() Ajax health: 12 Damage: 5
In operator -= after if() Ajax health: 7 Damage: 5
In operator -= before if() Hector health: 20 Damage: 5
In operator -= after if() Hector health: 15 Damage: 5
In operator -= before if() Ajax health: 7 Damage: 5       // Look closely here.
In operator -= after if() Ajax health: 0 Damage: 5          // and compare it to here.
Winner is Hector in 3 rounds.   // This appears to be correct, only 3 calls to this function for Ajax and 3 calls for Hector.



cire (8284)
Anyone who knows why this rounds decrement by 1 not in every case but it appears in some cases?


Considering that you didn't seem to grasp the significance of my last post, let's reduce your code to something that clearly illustrates a problem:

Change main to look like this:
1
2
3
4
5
6
7
8
9
10
11
 
#include <iostream>
#include "Hero.h"

int main() {

    sict::Hero a("a", 6, 0);

    std::cout << "Before: " << a << '\n';
    a -= 4;
    std::cout << "After: " << a << '\n';
}


And the overloaded ostream operator like so:
1
2
3
 
    ostream& operator<<(ostream& os, const Hero& hero) {
        return os << hero.m_name << ", health: " << hero.m_health ;
    }


What is the result of subtracting 4 from 6?
Before: a, health: 6
After: a, health: 0


Looks to be 0. Trace through the logic of the function. You'll figure it out.
lithony (26)
How come it does extra damage when it supposes to set it to 2 ?
cire (8284)
Trace through the logic of your operator-= paying close attention to what the value of m_health would be on each line. You'll figure it out.
lithony (26)
the logic of -= operator is

if the attack is above 0, the health will be deducted amount of attack.

if the attack is greater than m_health, it makes it 0 so that if health is 6 and attack is 7, it should instantly kill the other hero. But I still don't get how attack is 4 and health is 6 and health becomes 0. because the only condition for health to become instantly 0 is when attack is higher number than health
lithony (26)
Nvm, got it to work. I put else statement instead of another if in -= operator. is this how you wanted me to do it?
cire (8284)
is this how you wanted me to do it?

There are a number of ways you can do it right. What I wanted you to do was trace through the logic so you could understand why it's wrong.

1
2
3
4
5
6
7
8
 
    void Hero::operator-=(int attack) {
        if (attack > 0) {
            m_health -= attack;
        }
        if (attack > m_health) {
            m_health = 0;
        }
    }


When m_health starts at 6 and attack is 4, attack is greater than 0, so m_health is reduced by attack which means when line 3 concludes, m_health is 2.

Then, attack (4) is greater than m_health (2), so m_health is set to 0.

It could be written as you say you've corrected it, or it could be written:
1
2
3
4
 
    void Hero::operator-=(int attack) {
       m_health -= attack;
       if (m_health < 0) m_health = 0;
    }


Or you could just not bother with keeping m_health at 0 or above and interpret dead as 0 or less.
Last edited on
Pages: 12