Simple unit converter in C

Question

I purchased "A Book on C" for my procedural programming class and I was going through some of the exercises. Chapter 2 Exercise 9 is about designing a unit converter that can work with ounces, pounds, grams, and kilograms.

The code I've written works, but I really think it could be done much cleaner. Using nested if statements seems like a messy way to go about this.

Also, one problem I noticed was that if a char or string is given to scanf() on line 27, it will persist and then be passed to the scanf() on line 95. For example, if you enter "y" as the value to convert, the program will goto beginning without allowing the user to answer "Would you like to perform additional conversions?" How can I go about fixing this so that if a NaN is input it is discarded?

My code can be located at: http://pastebin.com/4tST0i7T

Answer 1

The most reliable way is to read input string using fgets() function, check if it contains digit using isdigit() (all characters in string) and then convert it to numeric value using atoi().

BTW, the last two operations can be replaced by strtol().

Answer 2

There are rare situations where the use of goto is a good choice (error handling for example) but your program certainly does not fall into this category, it looks like a bad spaghetti code.

Answer 3

One way to clean up the if structure would be to convert the value from the "fromUnit" to a common value and then convert it to the "toUnit". It simplifies the structure by leaving only two if structures around. (It also scales better.) So, it would be something more like:

if (!strcmp(fromUnit, "pound")) {
tempval = input / 16;
} else if (!strcmp(fromUnit, "gram") == 0) {
tempval = input * OUNCESTOGRAMS;
}

if (!strcmp(toUnit, "pound")) {
output = tempval * 16;
} else if (!strcmp(toUnit, "gram")) {
output = tempval / OUNCESTOGRAMS;
}

Granted, that math isn't correct, it's just there for the example. You would just have to (1) pick the temporary unit that you wanted to use (2) convert from the input unit to that unit and (3) convert from the temporary unit to the output unit.

And as someone else mentioned, gets() is definitely the way to go.

Answer 4

I would do it something like this:

#include <stdio.h>

typedef struct _unit {
    char * name;
    float grams;
} unit; 

unit units[] = {
    {"gram", 1.0},
    {"kilogram", 1000.0},
    {"pound", 500.0},
    {"ounce", 28.3495231}
};

unit * search_unit(char * name)
{
    int i;
    for (i = 0; i < (sizeof(units) / sizeof(unit)); i++)
    {
        printf("%d %s\n", i, units[i].name);
        if (0 == strcmp(units[i].name, name))
        {
            return & units[i];
        }
    }
    return NULL;
}
int main() {
    char line[10];
    char unitname[10];
    int number;
    unit * found_unit;

    while (1)
    {
        fgets(line, sizeof(line), stdin);
        if (1 == sscanf(line, "%d", &number))
        {
            break;
        }
        printf("not a number\n");
    }

    while (1)
    {
        fgets(line, sizeof(line), stdin);
        sscanf(line, "%s\n", unitname);
        found_unit = search_unit(unitname);
        if (found_unit)
        {
            printf("%d %s is %f grams\n", number, unitname, found_unit->grams * number);
            break;
        }
        printf("unknown unit\n");
    }
}

• Store your data in some data structure, instead of in the code.
• First read a line of text, then check whether it is a number.
• When reading from stdin, take the size of the buffer into account.
• Use loops instead of goto's.
• Use some common unit, grams for example, to calculate anything to anything.