2
#include <iostream>
#include <cmath>
using namespace std;

string number = "159";
float valueFloat = 0;
int valueInt = 0;
int main()
{
    for(int i = number.length()-1; i >=0; i--)
    {
        valueInt += (number[i]-48) * pow(10,i);
        valueFloat += (number[i]-48) * pow(10,i);
    }
    cout<<"Int value: "<<valueInt<<endl;
    cout<<"Float value: "<<valueFloat<<endl;
    return 0;
}

Output:

Int value: 950
Float value: 951

Why this code is returning different values? Why Int type is wrong there?

gsamaras
  • 71,951
  • 46
  • 188
  • 305
  • 1
    http://cpp.sh/7nlp – Humam Helfawi Mar 16 '16 at 07:43
  • Because `pow` calculates a floating point result, that gets truncated when converting to int. – Mats Petersson Mar 16 '16 at 07:44
  • Next time say why you would expect the output to be different, I will upvote though, since you are still new here. – gsamaras Mar 16 '16 at 08:00
  • "Why Int type is wrong there?" -- It *never is wrong*. The program does *exactly* what you told it to do. If results are not what you expected, you didn't tell it what you thought you did. (Finding faults in compilers or standard libraries is *highly* unlikely, especially for beginners.) ;-) – DevSolar Mar 16 '16 at 09:16

3 Answers3

2

It is not consistent behaviour. It may return the right number even in int on some platforms. The problem is in pow(10,i) this function return a float number and when add it to other int (or char) it would return a another float number. Then you are adding the whole number to an int variable which means that a casting from float to int is necessary. The number that you are getting in float may be 951 or 950.99999 (or something similar). if 950.99999 is casted to int it would be 950. That explains the results.

if you want to stick to an int variable for some reason (which does not seem exist) you should do something like that instead:

for(int i = number.length()-1; i >=0; i--)
{
    float p_float=pow(10,i);
    int p_int=static_cast<int>(p_float);
    p_int=(p_int-p_float)>.5f?p_int+1:p_int;
    valueInt += (number[i]-48) * p_int;
    valueFloat += (number[i]-48) * p_float;
}

Or as @Mats Petersson mentioned you can just use std::lround:

for(int i = number.length()-1; i >=0; i--)
{
    valueInt += (number[i]-48) * std::lround(pow(10,i));
    valueFloat += (number[i]-48) * pow(10,i);
}
Humam Helfawi
  • 19,566
  • 15
  • 85
  • 160
  • Or use `round` - but I would simply "accumulate" an integer multiplier by starting on 1 and multiplying by 10 each time - most likely faster [even for floating point values] than calling `pow`. – Mats Petersson Mar 16 '16 at 08:03
  • Is not round return a float? However, the other solution is better but the case may not be alwayse about pow – Humam Helfawi Mar 16 '16 at 08:03
  • You may meant to mention std::lround. Yes it is better. – Humam Helfawi Mar 16 '16 at 08:05
  • `round` returns a float rounded to the nearest integer. So, assuming `pow` actually returns something reasonably close to the correct value, it would give the right value as an integer result [and it should for any value valid as both integer and float, once you get out of the `float` range it may go wrong]. – Mats Petersson Mar 16 '16 at 08:19
1

Even though I couldn't re-generate what you see with gcc 4.9.2, I added some printfs in the loop, like this:

printf("%d\n", pow(10, i));
printf("%f\n", pow(10, i));

and got:

gsamaras@pythagoras:~$ g++ -Wall pc.cpp 
pc.cpp: In function ‘int main()’:
pc.cpp:14:27: warning: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘__gnu_cxx::__promote_2<int, int, double, double>::__type {aka double}’ [-Wformat=]
  printf("%d\n", pow(10, i));
                           ^
gsamaras@pythagoras:~$ ./a.out 
1076101120
100.000000
1076101120
10.000000
1076101120
1.000000
Int value: 951
Float value: 951

I suspect that you see this behavior because of the pow(), which has this signature (closest to int):

float pow (float base, float exponent);

As a result, pow() returns a float which is then been converted to int, which may result in loss of some accuracy.

gsamaras
  • 71,951
  • 46
  • 188
  • 305
1

The problem is that pow calculates the value not as an integer value (perhaps pow(x, y) is calculated exp(ln(x)*y) - but it doesn't HAVE to be exactly that way), but as a floating point value, and it will have a small rounding error. This may be "below" or "above" the correct value, and since conversion from float to int is by truncation, 950.999996 gets converted to 950 rather than 951. You can solve that, in this case, by rounding the result of pow to the nearest rather than truncating [assuming the result is less than +/-0.5 off from the infinitely precise value].

The way floating point values are calculated is ALWAYS with a degree of precision. Integer calculation is always exact to the least integer value of the calculation itself, but of courwse, the rules of C and C++ is that if one side is floating point, the calculation is performed in floating point.

I would completely eliminate the use of (often troublesome) pow from the code. A much better way, both for calculating the correct value and speed, is to use a multiplier value that is updated each loop iteration. Something like this:

int imult = 1;
float fmult = 1.0f;
for(int i = number.length()-1; i >=0; i--)
{
    valueInt += (number[i]-48) * imult;
    imult *= 10;
    valueFloat += (number[i]-48) * fmult;
    fmult *= 10.0f;
}

[I'm showing both integer and float multiplier, not strictly required, you'd get just as good an answer using imult in the second loop - but it becomes a floating point value either way, so I thought I'd show both variants]

For the valid range of int and valid range of float, this should give a more accurate result. Of course, integer values in float are restricted to 23 bits before they start getting chopped because of lack of precision available in floating point format. So any value above around 8.3 million will be missing the least significant bit.

Mats Petersson
  • 126,704
  • 14
  • 140
  • 227