cout issue maybe?

[Exempt]

I'm not sure how to describe the title so sorry about that one but my issue is wierd..

I use this code as I iterate through a std::map I have full of stat data. Exp PH = Exp per hour it works perfect. AVG G = Average gains per skill increase. SG PH = Skill Gains per hour... this always read 0 even though I've tested the variables used many times and they are working fine infact they are used in the first 2 couts.

second.total = total exp gained
second.ticks = number of times I've gained exp in the skill
seconds is just a int holding GetTickCount() - startTickCount.

All of these variable are working correctly but (i->second.ticks / seconds)*(60*60) always equals 0. second.ticks is 1 or more it's never 0.

                cout << i->first
                    << fixed << "  Exp PH: " << (i->second.total / seconds)*(60*60)
                    << "  AVG G: " << (i->second.total / i->second.ticks)
                    << "  SG PH: " << (i->second.ticks / seconds)*(60*60)
                    << endl << endl;

edit: first(the map key) is just the skill's name as a string.

[Disch]

Looks like integer division.

Assuming you have the below values:

int ticks = 15;
int seconds = 20;

cout << (ticks/seconds)*(60*60); // outputs:  0 

The problem here is that ticks and seconds are both integers, so the result when you divide them is going to be an integer.

15/20 gets truncated to the floor integer value... in this case: 0

0 * (60*60) is still 0.


To solve, do the multiplication first to avoid premature truncation:

cout << (ticks * 60*60 / seconds); // outputs what you'd expect 

[xkcd83]

Better still, properly cast it.
Since you are using C++, use a static_cast<float> instead of a C-style casting.
Doing the multiplication first might give you the correct answer, but I would say that in general this is bad programming practice.

[Exempt]

Thanks for the replies I will give them a try.

Edit: Using only added a few 0's to the decimal place and didn't change the result.
static_cast<float>(i->second.ticks / seconds)*(60*60)

Edit2: Disch's way does work and I don't really see a reason why I shouldn't use it that way? What makes it bad practice?

[Disch]

Better still, properly cast it.

There's no need to cast it to a double/float if he wants an integer. (edit: at least I didn't think so, but I might be wrong)

Casting to a double/float, then back to an integer is more expensive than keeping it an integer the whole time.

Doing the multiplication first might give you the correct answer, but I would say that in general this is bad programming practice.

I would say the exact opposite. (usually)



EDIT:

Using only added a few 0's to the decimal place and didn't change the result.

You're casting after the division is performed so you still get truncated. To do what xkcd83 is suggesting you'd have to cast one or both of the values before the division takes place.

Disch's way does work and I don't really see a reason why I shouldn't use it that way? What makes it bad practice?

The downsides to the approach I recommended are:

1) readability (subjective, but some people might think it's harder to read. Personally I think it's easier to read than casts)

2) Risk for "overflowing". If "ticks" is >= 596524, you will exceed the capacity for a 32-bit signed integer and will get an incorrect result.


So yeah if "ticks" is going to be a higher value, you might want to consider the casting approach instead. I had assumed this wasn't the case because you kept getting 0, so I thought the numbers were always small.