Convert and print Celsius to Fahrenheit from certain range of values

Question

I just started learning C, and got to the "for loops"—where an exercise is given to convert Celsius to Fahrenheit ( from -50 to 1000 with a step of 20 ), and print the values. I decided to play with it by adding different steps when certain values are reached.

I do have some programming experience ( Python/Java/JavaScript ) but it's all in beginner level scope.My concern is that i could have used one loop and check each (bp) with a condition... or maybe use module operator to determine a step change ... or something third. I need an expert opinion on the code i wrote.

Is this a good approach for this particular problem?

Is this considered bad or good practice?If bad what would you change and why?

Would you (assuming as an expert) go with this or other way ? If other, why ?

#include <stdio.h>

int main(){

    float fahr, cels, step, min, max;

    float count[4] = {10,1,10,100};   
    float bp[4] = {-50,-10, 10, 100};

    size_t bp_size, st_size;
    int c,c2;

    bp_size = *(&bp + 1) - bp; 
    st_size = *(&count + 1) - count;    

    min = -50; max = 1000;
    step = count[0];
    c2 = 0;

    for (cels = min; cels <= max; cels += step){

        fahr = cels * 1.8 + 32;
        printf("%4.0f C\t= %6.1f F\n", cels, fahr);

        for(c = 0; c < bp_size; c++){
            if (cels == bp[c]){ 
                if (c2 == st_size){ c2 = 0;}
                step = count[c2];
                c2++;
            }   
        }
    }   
}

Answer 1

The program works correctly, but there are a number of things you might be able to improve here. Here are some observations in no particular order.

Reduce the number of variables

There are a lot of variables here, which makes it a bit difficult to read and understand the program. Some of the suggestions below will show some ways to reduce that number and add more clarity to the program.

Use const where practical

A number of places in the code should have the const keyword added. For example instead of this:

float count[4] = {10,1,10,100};   
float bp[4] = {-50,-10, 10, 100};

write this:

const float count[4] = {10,1,10,100};   
const float bp[4] = {-50,-10, 10, 100};

This indicates, both to the compiler and to human readers of the code, that these values will not be altered.

Use more meaningful variable names

The variable names step, min and max are useful and descriptive of what those variables actually do. However, count and bp are not very good names. I'd suggest calling them something more closely related to their function as with the following suggestion.

Consider using a struct

The count and bp values are closely related, but this is not clearly indicated in the code. It would be easier to understand, I think, if a structure were used instead:

struct {
    float startTemp;
    float stepSize;
} const limits[] = {
    { -50,  10 },
    { -10,   1 },
    {  10,  10 },
    { 100, 100 },
    {1000,   0 },
};

Note that the minimum and maximum values are built into the table so there isn't any need for additional variables for those uses.

Declare variables in the smallest practical scope

By declaring variables in the smallest practical scope, you reduce the chance for name collisions and make it clear to the reader of your code where variables are and are not needed. With any C compiler conforming to the 1999 specification (which should be all of them at this point!) one could rewrite the outer for loop like this:

for (float cels = min; cels <= max; cels += step) {
    float fahr = cels * 1.8 + 32;
    /* etc.*/
}

Initialize variables when they are declared

If you follow the advice above about reducing the scope of variables, you can also easily initialize variables when they are declared. This helps to make sure that variables aren't used with uninitialized values.

Declare each variable on its own line

Declaring variables like this:

float fahr, cels, step, min, max;

is not good current practice. Instead, declare one variable per line and initialize it.

Use the sizeof operator

These two lines are quite peculiar:

bp_size = *(&bp + 1) - bp; 
st_size = *(&count + 1) - count;    

A better way to find the size of such arrays would be this:

size_t bp_size = sizeof(bp);

Don't test floating point numbers for equality

One of the lines in the current code is this:

if (cels == bp[c]){ 

It will probably work just fine in this context, but generally, one should avoid testing floating point numbers for equality. It's usually a better bet to test instead for < or <=. See this question for more details.

Put if and for code on separate line(s)

The code contains this line:

if (c2 == st_size){ c2 = 0;}

It's not technically wrong and the compiler will accept it, but it's too easy for a human reader to overlook the contents of the if when it's hiding at the end of the same line. Prefer instead to write it like this:

if (c2 == st_size) { 
    c2 = 0;
}

Think carefully about loops

In this code, the outer loop seems clear enough, but the inner loop is not. The point of it is to determine what step size to use but the code to do this is more complex than it needs to be:

for(c = 0; c < bp_size; c++){
    if (cels == bp[c]){ 
        if (c2 == st_size){ c2 = 0;}
        step = count[c2];
        c2++;
    }
}

First, we could just keep track of the step size as we go instead of searching for it each time. Second, once the step size is found, there's no reason to continue through the rest of the loop.

Here's an alternative implementation that uses all of these ideas:

int main(){
    struct {
        float startTemp;
        float stepSize;
    } const limits[] = {
        { -50,  10 },
        { -10,   1 },
        {  10,  10 },
        { 100, 100 },
        {1100,   0 },
    };

    for (int i=0; limits[i].stepSize != 0; ++i) {
        for (float cels = limits[i].startTemp;
            cels < limits[i+1].startTemp;
            cels += limits[i].stepSize) 
        {
            float fahr = cels * 1.8 + 32;
            printf("%4.0f C\t= %6.1f F\n", cels, fahr);
        }

    }
}

Note that the last item in the list is 1100 degrees rather than 1000. This is done because we want to include the value of 1000 in the list.