Thread: Need someone to check my code

Hi,
I solved the 1st problem on CProgramming. However, when I checked the solution, it was very much different from my code. When I run my program it gives the expected result all right. So, I need someone to check my code and see if it can be considered the 'correct' solution to the problem. Also, can you please tell me the parts of my code that I can improve?
(Note: I am new to C. Just started it a while ago.)

Code:

Code:

#include <stdio.h>

//Function that converts from degreeCelsius to degreeFahrenheit
float convert_to_degf(float deg_c)
{
    float result = ((deg_c*9)/5)+32;
    return result;
}
//The main function
int main()
{
    //Variable declarations
    int low_lim = -1;
    int high_lim = -1;
    int step = -1;
    float result;
    //Get the lower limit from user. Repeat if not within the range.
    while(low_lim < 0)
    {
        printf("Please give in a lower limit, limit >= 0: ");
        scanf("%d", &low_lim);
    }
    //Get the higher limit from the user. Repeat if not within the range.
    while(high_lim <= low_lim || high_lim > 50000)
    {
        printf("Please give in a higher limit, %d < limit <= 50000: ", low_lim);
        scanf("%d", &high_lim);
    }
    //Get the step from the user. Repeat if not within the range. 
    while(step < 0 || step > high_lim-low_lim)
    {
        printf("Please give in a step, 0 < step <= %d: ", high_lim-low_lim);
        scanf("%d", &step);
    }
    //Prints the table headings.
    printf("\nCelsius               Fahrenheit\n");
    printf("-------               ----------\n");
    //Print the values till the next value is greater than the high limit.
    while(1)
    {
        result = convert_to_degf(low_lim);
        printf("%d.000000             %f\n", low_lim, result);
        if(low_lim+step <= high_lim)
        {
            low_lim += step;
        }
        else
        {
            break;
        }
    }
    //Wait for user to press a key
    getch();
    return 0;
}

Thanks.

The new version looks much better. A few other notes, mostly stylistic issues:

• As a convention, names of constants and #defines are usually ALL_CAPS, which makes for a nice visual reminder that it's a constant/#define when reading & maintaining code. Readability and maintainability is of utmost importance as your projects grow larger.
• I would use doubles instead of floats, they're much more precise. I only use floats if I absolutely have to, e.g. for size reasons, which is rare.
• Since you made absolute_zero a constant, it would make more sense to initialize low_limit to absolute_zero-1 and high_limit to low_limit-1, so if you change absolute_zero your loops would still work. That's not a real issue here (the only change to absolue_zero might be to make it -273.15, which would still let your input loops run), but it would better convey the intention of the code. I.e. the code is setting low_lim to a value below absolute_zero and high_lim to a value below low_lim, so your loops will not be skipped. There is a better way though: use a do-while loop. Then you don't need to worry about initial values for your user input variables. The loop condition is checked after the loop body is executed, so the variable has the value the user input, and you don't need to worry about giving it the right initial value (this is more beneficial if you ever end up changing the loop condition significantly). Also, they are guaranteed to run at least once, which is good since you always want to ask the user at least once for input, and only repeat if it's invalid.
• You should get in the habit of checking return codes of functions, in particular IO functions (user, file, network, etc) like scanf. It should return the number of values read. If it's not what you were expecting (1 in each of your cases, for the single %f/%d in each call), handle the error and stop or retry as appropriate.
• You may want to clear the input buffer after each scanf call. If the user accidentally inputs some letters, your loop may get stuck. There's an example here: FAQ > Flush the input buffer - Cprogramming.com. The key part is that while (getchar()) loop, which, if you put into a function (e.g. called flush_input_buffer()), make a nice one-line way to clear the input buffer.
• If you expect a user to press a key to continue/terminate the application, it's nice to inform them. Sure, they'll probably hit one anyway, but best be safe. Also, if all you're doing there is trying to keep your IDE from closing the program before you can read the output, there are better ways, that don't annoy regular terminal users (like me).

Originally Posted by anduril462

The new version looks much better. A few other notes, mostly stylistic issues:

• As a convention, names of constants and #defines are usually ALL_CAPS, which makes for a nice visual reminder that it's a constant/#define when reading & maintaining code. Readability and maintainability is of utmost importance as your projects grow larger.
• I would use doubles instead of floats, they're much more precise. I only use floats if I absolutely have to, e.g. for size reasons, which is rare.
• Since you made absolute_zero a constant, it would make more sense to initialize low_limit to absolute_zero-1 and high_limit to low_limit-1, so if you change absolute_zero your loops would still work. That's not a real issue here (the only change to absolue_zero might be to make it -273.15, which would still let your input loops run), but it would better convey the intention of the code. I.e. the code is setting low_lim to a value below absolute_zero and high_lim to a value below low_lim, so your loops will not be skipped. There is a better way though: use a do-while loop. Then you don't need to worry about initial values for your user input variables. The loop condition is checked after the loop body is executed, so the variable has the value the user input, and you don't need to worry about giving it the right initial value (this is more beneficial if you ever end up changing the loop condition significantly). Also, they are guaranteed to run at least once, which is good since you always want to ask the user at least once for input, and only repeat if it's invalid.
• You should get in the habit of checking return codes of functions, in particular IO functions (user, file, network, etc) like scanf. It should return the number of values read. If it's not what you were expecting (1 in each of your cases, for the single %f/%d in each call), handle the error and stop or retry as appropriate.
• You may want to clear the input buffer after each scanf call. If the user accidentally inputs some letters, your loop may get stuck. There's an example here: FAQ > Flush the input buffer - Cprogramming.com. The key part is that while (getchar()) loop, which, if you put into a function (e.g. called flush_input_buffer()), make a nice one-line way to clear the input buffer.
• If you expect a user to press a key to continue/terminate the application, it's nice to inform them. Sure, they'll probably hit one anyway, but best be safe. Also, if all you're doing there is trying to keep your IDE from closing the program before you can read the output, there are better ways, that don't annoy regular terminal users (like me).

Thanks for the tips. I'll keep them in mind .