Float value not being stored correctly

Question

I'm writing a function that's suppose have the float "length" passed to it and then display a timer like output. The float "length" is meant to be in minutes.

When I run it, the output should be: 02:03:27:00. Instead it displays 02:03:26:100, while being technically correct it's A) Not how it should displayed and B) Shows that there's a bug somewhere that may cause undesired results in the future.

After going through it manually with a calculator and finding all the math to be sound. I then commented out the parts that formats the zeros to see if that was causing the bug and the problem was still there. I then went through putting printf's after every calculation and found that while "length" is set to 123.45 it's being stored as 123.449997?

I haven't a clue way it's doing this. And because I don't know way or how this is happening I can't write a reliable fix for it.

int main()
{

float length;
float working;
int hour;
int min;
int sec;
int centi_sec;
char hzero[2];
char mzero[2];
char szero[2];
char czero[2];

    length=123.45;
    working=floor(length);
    working=(length-working)*60;
    sec=floor(working);
    working-=floor(working);
    centi_sec=(working*100)+.5;
    working=floor(length);
    hour=floor((working/60));
    min=working-(hour*60);
    if(hour<10){
        hzero[0]='0';
        hzero[1]="";
    }
    else if(hour==0){
        hzero[0]='0';
        hzero[1]='0';
    }
    else{
        hzero[0]="";
        hzero[1]="";
    }
    if(min<10){
        mzero[0]='0';
        mzero[1]="";
    }
    else if(min==0){
        mzero[0]='0';
        mzero[1]='0';
    }
    else{
        mzero[0]="";
        mzero[1]="";
    }
    if(sec<10){
        szero[0]='0';
        szero[1]="";
    }
    else if(sec==0){
        szero[0]='0';
        szero[1]='0';
    }
    else{
        szero[0]="";
        szero[1]="";
    }
    if(centi_sec<10){
        czero[0]='0';
        czero[1]="";
    }
    else if(centi_sec==0){
        czero[0]='0';
        czero[1]='0';
    }
    else{
        czero[0]="";
        czero[1]="";
    }
    printf("%s%d:%s%d:%s%d:%s%d\n", hzero, hour, mzero, min, szero, sec, czero, centi_sec);
    system("pause");

}

I've also written a short program just to cheek that the problem wasn't being influence by something in the full program that I had missed and it's having the same issue.

int main()
{

float length=123.45;

    printf("%f\n", length);
    system("pause");

}

P.S. When I was using printf to troubleshoot the problem, I discovered that the printf's were messing up the formatting of the zeros. Not too big of a issue since when I removed them the formatting went back to how it should be. Still it doesn't make any sense that printf's would be messing up the formatting. If anyone can provide an answer to this too, it would be appreciated.

Thanks in advance.

You have a potential problem because `floor()` does not return `int` it returns `double`. Also isn't this `hzero[0]="";` generating compiler warnings? — Iharob Al Asimi, Jun 27 '15 at 20:34

score 1 · Answer 1 · edited May 23 '17 at 12:30

1

A binary representation of a decimal fraction can not be precise in all cases for mathematical reasons. You can read more about this here

To avoid the problem, you need to add half of your smallest unit to the input. This would be 1.0/60/100/2 in this case.

length = 123.45;
const float epsilon = 1.0/60/100/2;
length += epsilon;

You have tried to do something similar with

centi_sec=(working*100)+.5;

but this has only an effect on centi_sec, and not on the other numbers. Change it back to

centi_sec=(working*100);

Please also change the array sizes as suggested by @iharob.

Edit: You can avoid the arrays altogether:

#include <math.h>
#include <stdio.h>

int main()
{

const float epsilon = 1.0/60/100/2;
float length;
float working;
int hour;
int min;
int sec;
int centi_sec;

    length=123.45;
    length += epsilon;
    working=floor(length);
    working=(length-working)*60;
    sec=floor(working);
    working-=floor(working);
    centi_sec=(working*100);
    working=floor(length);
    hour=floor((working/60));
    min=working-(hour*60);
    printf("%02d:%02d:%02d:%02d\n", hour, min, sec, centi_sec);
//    system("pause");

}

This works.

edited May 23 '17 at 12:30

Community

1
1

answered Jun 27 '15 at 20:47

alain

11,939
2
31
51

That actually makes it less accurate then before. – Ulrick Jun 27 '15 at 21:30
I made a simpler version of your program, it works for me. – alain Jun 27 '15 at 21:49
The problem was that float was stored as 123.449997, in your code it's stored as 124.450081, which is less accurate then mine. But it does in fact completely remove the need for the zero arrays. So it is a big improvement in that aspect. – Ulrick Jun 27 '15 at 22:01
Now I see what you meant with "less accurate". This is needed to avoid problems like printing 02:03:26:100 instead of 02:03:27:00. You can of course still store 123.45 everywhere, the +epsilon is needed only when formatting for display. – alain Jun 27 '15 at 22:14

score 1 · Accepted Answer · answered Jun 27 '15 at 20:54

You have assigned a binary floating point value with a decimal real value. Binary floating point cannot represent exactly all real decimal values.

Single precision binary floating point is good for precise representation of approximately 6 decimal significant figures, and 123.449997 is 9 figures; so you have exceed the promised precision. By default the %f format specifier displays 6 decimal places, but in this case that exceeds the available precision.

Either use a format specifier that displays to reasonable precision:

printf( "%.3f\n", length ) ;

or use double which is good for 15 decimal significant figures.

There are few reasons not to use double precision on a target with high memory bandwidth and a hardware floating point unit (i.e all modern desktop computers). Single precision is useful if you are processing truly huge amounts of data and need to reduce processing time and don't need the precision.

Iharob Al Asimi · Answer 3 · 2015-06-27T20:57:17.503

The problem in your code has nothing to do with floating point precision, there is a limit on precision and because of that rounding artihmetic operations introduces errors, but for two decimal places there should be no problem, and it doesn't matter if internally the value is printed as 123.449997, if you do

printf("%.2f\n", 123.449997);

123.45 will be printed, and as well for any arithmetic operation involving the value, the result will be correct for enough decimal places of accuracy, 2.

The most important problem you have is that your strings cannot be strings like that, because there is no room for the terminating '\0'.

And the math is wrong too, because if centi_sec is larger or equal to 100 then one second should be added and 100 should be subtracted from centi_sec, and the same applies to sec with min, and so on.

These

char hzero[2];
char mzero[2];
char szero[2];
char czero[2];

should read

char hzero[3];
char mzero[3];
char szero[3];
char czero[3];

You should also not repeat yourself, use a function

#include <math.h>
#include <stdio.h>

void zeropad(int value, char str[3])
 {
    if (value < 10)
     {
        str[0] = '0';
        str[1] = value + '0';
     }
    else
     {
        str[0] = (value - value % 10) / 10 + '0';
        str[1] = value % 10 + '0';
     }
    str[2] = '\0';
 }

int main()
{
    float length;
    float working;
    int hour;
    int min;
    int sec;
    int centi_sec;
    char hzero[3];
    char mzero[3];
    char szero[3];
    char czero[3];

    length    = 123.45;
    working   = floor(length);
    working   = (length - working) * 60;
    sec       = floor(working);
    working  -= floor(working);
    centi_sec = (working * 100) + .5;
    working   = floor(length);
    hour      = floor(working / 60);
    min       = working - (hour * 60);

    if (centi_sec >= 100)
     {
        sec       += 1;
        centi_sec -= 100;
     }

    if (sec >= 60)
     {
        min += 1;
        sec -= 60;
     }

    if (min >= 60)
     {
        hour += 1;
        min  -= 60;
     }

    zeropad(hour, hzero);
    zeropad(min, mzero);
    zeropad(sec, szero);
    zeropad(centi_sec, czero);

    printf("%s:%s:%s:%s\n", hzero, mzero, szero, czero);
}

That may be a problem with the code posted, but it is not related to the question asked, and should be posted as a comment if at all. — Clifford, Jun 27 '15 at 20:42
The issue with length is also present in your example, but void zeropad does fix the issue with printf messing up the formatting. I'm still know to c and I don't quite understand everything that's going on in zeropad. I get some of it but not everything. Also I still can't understand why printf would be messing up the formatting in the first place? — Ulrick, Jun 27 '15 at 21:27

score 0 · Answer 4 · edited Jun 27 '15 at 20:59

Floating point numbers have only so much accuracy and not all numbers can be exactly represented in floating point variables.

My first suggestion is to pass a small struct that contains 4 integer fields

int hours
int minutes
int seconds
int fractionSecondX100

So as to avoid the whole problem set with floats.

However, if you must use float values, then google: "how to handle float values in C", this will return many 'hits' following the google, read several of the referenced web pages so you know how to handle floats and have a knowledge of what to expect.

Float value not being stored correctly

4 Answers4

Linked