Ulrick
Ulrick

Reputation: 33

Float value not being stored correctly

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.

Upvotes: 1

Views: 2804

Answers (4)

alain
alain

Reputation: 12047

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.

Upvotes: 1

user3629249
user3629249

Reputation: 16540

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.

Upvotes: 0

Iharob Al Asimi
Iharob Al Asimi

Reputation: 53006

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);
}

Upvotes: 0

Clifford
Clifford

Reputation: 93476

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.

Upvotes: 1

Related Questions