HiveBrain v1.2.0
Get Started
← Back to all entries
patterncModerate

Simple temperature converter in C

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
simpletemperatureconverter

Problem

I have just begun to learn the C language and wanted to start off small with a temperature conversion calculator. The program works, and I'm looking for feedback on do's and don'ts as I am sure there are at least a few errors in technique that can be improved. (I just realized Fahrenheit is spelled incorrectly).

```
/Calculates user provided temperature in Farenheight or Celsius depending on option chosen./

#include "stdafx.h" //don't actually know what this is for however will not compile without it.
#include
#include
#include

/Calculator options/
#define option_1 1
#define option_2 2
#define option_3 3
#define option_4 4

void welcome();
void calculator();
void get_input(_Bool, int, float*);
void display_result(int, float);
float F_to_C(float);
float C_to_F(float);

int main(void)
{
welcome();
calculator();
_getch();
}

void welcome() /Welcome message + instruction/
{
printf("Welcome to the Temperaure Conversion Calculator\n\n");
printf("Please select an option:\n\n");
printf("Options:\t1)F to C\t2)C to F\t3)OFF\n\n");
}

void calculator() /Conversion Calclator/
{
int option = 0;
float input_temp = 0.0f;
_Bool calc_off = 0;

while (!calc_off)

{
get_input(&calc_off, &option, &input_temp);

if (option == option_1) /F to C/
{
display_result(option_1, input_temp);
}
else if (option == option_2) /C to F/
{
display_result(option_2, input_temp);
}
}

}

void get_input(_Bool calc_off, int option, float input_temp) /Retrieve user input*/
{
int trash = 0;
int data_check;
printf("Select option: "); scanf_s("%d", option); /Obtains option/
scanf_s("%c", &trash); /obtains newline character that will cause infinite loop/

while (option != 1 && option != 2 && option != 3) /incorrect option selection*/
{
display_result(option_4, option_4);
scanf_s("%d", option);
scanf_s("%c",

Solution

Use an enum instead of #define. So instead of:

#define option_1 1
#define option_2 2
#define option_3 3
#define option_4 4


You can use enum:

enum {F_to_C, C_to_F, Off} option_type; // I am leaving off the fourth option!


You will also have to modify other portions of your code to use enums.

Stop using so many if statements. Use switch-case statements instead. There are a few places that you do this in your code but for example:

if (option == option_1)
    printf("Farenheight to Celsius of %.1f F is: %.1f C\n", input_temp, F_to_C(input_temp));

else if (option == option_2)
    printf("Celsius to Farenheight of %.1f F is: %.1f F\n", input_temp, C_to_F(input_temp));

else if (option == option_3)
    printf("Calculator OFF\n");

else if (option == option_4)
    printf("Invalid option choice, please try again...\nOption: ");


Instead:

switch(option) {
    case F_to_C:
        printf("Farenheight to Celsius of %.1f F is: %.1f C\n", input_temp, F_to_C(input_temp));
        break;
     ...
     default:
         printf("Invalid option choice, please try again...\nOption: ");
         break;


Notice that we have a default. You can give garbage as option_4, more than just the value 4. You should try handling this best you can.

Don't use _Bool! Just use bool and use true and false instead of 1 and 0. So this:

_Bool calc_off = 0;


Should be:

bool calc_off = false;


But actually I don't really believe you need calc_off because you use pointers unnecessarily.

This:

int option = 0;
float input_temp = 0.0f;
_Bool calc_off = 0;

while (!calc_off)

{
    get_input(&calc_off, &option, &input_temp);

    if (option == option_1) /*F to C*/ // Rewrite this to use switch case!
    {
        display_result(option_1, input_temp);
    }
    else if (option == option_2) /*C to F*/
    {
        display_result(option_2, input_temp);
    }
}


get_input in particular is troublesome.

Why not let get_input return an option? then:

while((option = get_input(...)) != Off) {

Code Snippets

#define option_1 1
#define option_2 2
#define option_3 3
#define option_4 4
enum {F_to_C, C_to_F, Off} option_type; // I am leaving off the fourth option!
if (option == option_1)
    printf("Farenheight to Celsius of %.1f F is: %.1f C\n", input_temp, F_to_C(input_temp));

else if (option == option_2)
    printf("Celsius to Farenheight of %.1f F is: %.1f F\n", input_temp, C_to_F(input_temp));

else if (option == option_3)
    printf("Calculator OFF\n");

else if (option == option_4)
    printf("Invalid option choice, please try again...\nOption: ");
switch(option) {
    case F_to_C:
        printf("Farenheight to Celsius of %.1f F is: %.1f C\n", input_temp, F_to_C(input_temp));
        break;
     ...
     default:
         printf("Invalid option choice, please try again...\nOption: ");
         break;
_Bool calc_off = 0;

Context

StackExchange Code Review Q#138661, answer score: 14

Revisions (0)

No revisions yet.