patterncModerate
Simple temperature converter in C
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",
```
/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
You can use
You will also have to modify other portions of your code to use
Stop using so many
Instead:
Notice that we have a default. You can give garbage as
Don't use
Should be:
But actually I don't really believe you need
This:
Why not let
enum instead of #define. So instead of:#define option_1 1
#define option_2 2
#define option_3 3
#define option_4 4You 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 4enum {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.