Converting latitude and longitude to float in C

Question

I'm trying to convert Lat and Lon coordinates to 4 decimal place float. I have 'working' code, but it's just a bit off like %50 of the time. Can anyone help me or have a better design to make it more exact?

 float sexag2decimal(char * deg){
    char *sdeg = malloc(sizeof(char)*3);
    char *smin = malloc(sizeof(char)*3);
    char *ssec = malloc(sizeof(char)*3);
    char *ssec2 = malloc(sizeof(char)*5);
    int size = strlen(deg);
    int m = 0;
    for(m = 0; m < size-1; m++){
        if(deg[m] >= 65 && deg[m] <= 122){
            return 0;
        }
    }
    float converted = 0;
    if(deg[0] == '0'){
        strcpy(&sdeg[0], &deg[1]);
        strcpy(&sdeg[1], &deg[2]);
        strcpy(&smin[0], &deg[4]);
        strcpy(&smin[1], &deg[5]);
        strcpy(&ssec[0], &deg[7]);
        strcpy(&ssec[1], &deg[8]);
        strcpy(&ssec2[0], &deg[10]);
        strcpy(&ssec2[1], &deg[11]);
        strcpy(&ssec2[2], &deg[12]);
        strcpy(&ssec2[3], &deg[13]);
    }else{
        if(deg[14] == 'W')
            return 0;
        strcpy(&sdeg[0], &deg[0]);
        strcpy(&sdeg[1], &deg[1]);
        strcpy(&smin[0], &deg[3]);
        strcpy(&smin[1], &deg[4]);
        strcpy(&ssec[0], &deg[6]);
        strcpy(&ssec[1], &deg[7]);
        strcpy(&ssec2[0], &deg[9]);
        strcpy(&ssec2[1], &deg[10]);
        strcpy(&ssec2[2], &deg[11]);
        strcpy(&ssec2[3], &deg[12]);
    }
    sdeg[2] = '\0';
    smin[2] = '\0';
    ssec[2] = '\0';
    ssec2[4] = '\0';
    converted = atoi(sdeg) + ((float)atoi(smin)/60.0) + (((float)atoi(ssec)+((float)atoi(ssec2))/10000)/3600.0);
    free(sdeg);
    free(smin);
    free(ssec);
    free(ssec2);
    return converted;
}

Thanks!

Input:

30-25-30.7140N, 086-53-37.8590W

29-57-33.3000N,081-20-23.0000W

My output:

30.4252,-86.8939

29.9592,-81.3397

Correct output:

30.4252,-86.8938

29.9593,-81.3397

Thanks!

Answer 1

OP's code suffers from attempting to write outside allocated buffers. @cyclaminist

float sexag2decimal(char * deg){
  char *sdeg = malloc(sizeof(char)*3);  // Too small for the entire string
  ...
    strcpy(&sdeg[0], &deg[0]); // UB: attempts to copy the _entire_ string to `deg`

Certainly OP meant to code something like

    // strcpy(&sdeg[0], &deg[0]);
    sdeg[0] = deg[0]; // Copy 1 char

---

it's just a bit off like %50 of the time

Given OP's code survived the above UB, the observed output differs from expectation in part due to the result of accumulated roundings in the calculation:

converted = atoi(sdeg) + ((float)atoi(smin)/60.0) + 
    (((float)atoi(ssec)+((float)atoi(ssec2))/10000)/3600.0);

This incurs rounding in (float)atoi(smin)/60.0, (float)atoi(ssec2))/10000), (float)atoi(ssec2))/10000)/3600.0) and each of the 3 additions.

Using higher precision math, like double rather than float, will dramatically reduce rounding effects, but not eliminate them.

To improve accuracy, perform the calculation with no rounding with integer math. The expected values of angle, in 10,000th of a second, needs about 36-bit math.

long seconds = ((atoi(sdeg) * 60) + atoi(smin)) * 60 + atoi(ssec);
long myriad_seconds = (seconds * 10000) + atoi(ssec2);

---

... to 4 decimal place float

This goal is somewhat a contradiction. float is very commonly employs a binary encoding and so does not produce a decimal. It does sound like OP want to print to 4 decimal places some float. To do so, code suffers additional roundings.

The conversion of the exact myriad_seconds to a float incurs a rounding error.

// "29-57-33.3000N"
// myriad_seconds = 1078533000
// With infinite math, the quotient is 29.95925
float converted = myriad_seconds/(60.0f * 60 * 10000);
// The "best" `float` is just less than 29.95925
// converted = 29.959249777...

The conversion of a float to 4 decimal place text incurs another rounding error.

printf("%.4f\n", converted);
// "29.9592"

If this is OP's goal, performing far fewer roundings like this code with greatly improve yet not eliminate the "once-in-a-while" off by one bit.

---

The originator of this task has cleverly chosen final values that are near xxx.xxxx5.... These values are sensitive to the computational path in being "just a bit off".

To counter this craftiness, consider instead of returning a float, return an integer of the number of 10,000th of a degree. With error/rounding free myriad_seconds and controlled rounding to myriad_degrees, code will always achieve the expected answer - at a cost of more complex source code.

// Add signed half of 3600 to effect rounding (half way away from zero) before division.
long myriad_degrees = (myriad_seconds + (myriad_seconds < 0 ? -1 : 1) * 3600 / 2) / 3600;

printf("%s%ld.%04ld\n", myriad_degrees < 0 ? "-" : "", 
    labs(myriad_degrees) / 10000, labs(myriad_degrees) % 10000);

// Output 
29.9593

Answer 2

(copied out of own comments)

First, the great part of time this function wastes seems to be in malloc. You have no reason to use dynamic heap at all: just allocate character arrays like

char sdeg[3];

Moreover, each return from function not from the last line causes 4 memory leaks (free is not called). Again, you don't need malloc at all.

Second, strcpy is inappropriate here: just copy the character. strcpy requires NUL-terminated string and copies full length of source. That's not the thing you need here.

Third: if you check input for saneness, excluding character range 65...122 isn't good idea. Particularly, you reject the case it's 'W', despite later you expect 'W' as a possible input character.

In general, I suggest using sscanf. With it, you can write code like:

if (sscanf(deg, "%d-%d-%d.%d", &n1, &n2, &n3, &n4) == 4) {
    return some_formula_on_n1...n4;
}

Finally, and the main issue: float accuracy is definitely not enough for your expectations. 1/10000 of a second (which in turn is 1/3600 of a degree) is 1/6,480,000,000 (of a full degree range 0...180) that is much smaller than float relative error (1/8,388,608). Switch to double in all values. Also, request to print at least 10 significant digits. 6 default ones are too small.

You could also consider the method described by @chux for only integer maths. Unlike binary floating, it's definitely free of rounding effects and hard-to-prove accuracy, despite more complex (and cumbersome).