Skip to content

Commit

Permalink
Clarify by-hand rounded %0.1f formatting
Browse files Browse the repository at this point in the history
Coverity is rightly pointing out that it is not clear that `double` math is wanted, where the conversion from `long long` to `double` should occur, and where integer remainders are discarded.

Most compilers will get it right, but humans may find it confusing or unexpected.

* Did some algebra to reduce half-bit errors.

* Added a cast to make the conversion explicit.

* Aligned turn expected damage with running expected damage so both are the same representation.

Closes CID 1380164
  • Loading branch information
GregoryLundberg committed Oct 31, 2017
1 parent 28cfa72 commit 520e617
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/gui/dialogs/statistics_dialog.cpp
Expand Up @@ -145,7 +145,7 @@ void statistics_dialog::add_damage_row(

std::ostringstream str;
str << damage << " / "
<< (expected * 10 + shift / 2) / shift * 0.1
<< static_cast<double>(((expected * 20) + shift) / (2 * shift)) * 0.1
<< " " // TODO: should probably make this two columns
<< ((dsa < 0) ^ (expected < 0) ? "" : "+")
<< (expected == 0 ? 0 : 100 * dsa / expected) << '%';
Expand All @@ -157,7 +157,7 @@ void statistics_dialog::add_damage_row(

if(show_this_turn) {
str << turn_damage << " / "
<< (turn_expected * 10 + shift / 2) / shift / 10
<< static_cast<double>(((turn_expected * 20) + shift) / (2 * shift)) * 0.1

This comment has been minimized.

Copy link
@jyrkive

jyrkive Oct 31, 2017

Member

For the record, I had already fixed the warning in this line (as there weren't any floating-point numbers left, see 670bcf7#diff-7c7efbed9f4ea67ab046385d27aa6d49)

This comment has been minimized.

Copy link
@GregoryLundberg

GregoryLundberg Oct 31, 2017

Author Contributor

By removing the fractional part, yes.

It seemed odd that one was NNN.N while the other was NNN and I liked the added .N instead.

Makes no difference to me, though, and I can revert that line and align the first to it, instead.

This comment has been minimized.

Copy link
@jyrkive

jyrkive Oct 31, 2017

Member

No need, it's fine for me this way.

<< " " // TODO: should probably make this two columns
<< ((dst < 0) ^ (turn_expected < 0) ? "" : "+")
<< (turn_expected == 0 ? 0 : 100 * dst / turn_expected) << '%';
Expand Down

0 comments on commit 520e617

Please sign in to comment.