Using functions to separate code for Fahrenheit to celsius conversion

Question

I'm relatively new to programming, and I've been wondering about a problem such as this. How can I think about decomposing the code into its constituent functions? I went the simplest route I could, just putting the formula in its own function. I believe this makes the code more readable, but there's also a part of me that wants to put the while loop into a function as well.

Is there a good rule of thumb for separating my code into functions?

#include <stdio.h>

int fahr_to_celsius(int f);

/* print Farhenheit-Celsius table
   for fahr = 0, 20, ..., 300 *;
   use a function for conversion */
int main(void)
{
  int fahr, celsius;
  int lower, upper, step;

  lower = 0; /* lower limit of the temperature table */
  upper = 300; /* upper limit */
  step = 20; /* step size */

  fahr = lower;
  while (fahr <= upper) {
    celsius = fahr_to_celsius(fahr);
    printf("%d\t%d\n", fahr, celsius);
    fahr = fahr + step;
    }
}

int fahr_to_celsius(int fahr)
{
  int celsius = 5 * (fahr-32) / 9;
  return celsius;
}

Answer 1

In a more complex program I would probably extract the loop, but I think this is actually pretty good as is. Your main routine reads very well and describes precisely what this program does. You separated the concerns of performing the conversion and displaying the results and your conversion method performs a single task. I agree that extracting it was a wise choice.

You'll often hear the phrase

A method should do one thing, and do it well.

You nailed the first part, but I'm not so sure about the second. I guess it depends on how accurate you need it to be.

  int celsius = 5 * (fahr-32) / 9;

This is very likely to be a non-integer number before you cast it. C casts from floating point to integer via truncation. This means that you're always "rounding" the result down. I doubt that's what you intended. Use the round() method instead for a more accurate result. Or, even better, return a double.

One other thing, don't abbreviate variable names. I understand why you used fahr instead of fahrenheit, but it's a bad habit to get into.

Answer 2

You're on the right track. I'd note that anything inside main() itself is not really set up for a unit test. You're thinking of making the inside of the while a function to break things up a little.

To me, I look at the main() and see it doing two things: setting up the parameters of the run and executing the run.

So you could do this:

#include <stdio.h>

double convert_fahrenheit_to_celsius(double fahrenheit);
void print_fahrenheit_celsius_table(FILE* out, int tableLowerLimit, int tableUpperLimit, int stepSize);

/* print Fahrenheit-Celsius table
for fahr = 0, 20, ..., 300 *;
use a function for conversion */
int main(void)
{
    int tableLowerLimit = 0;
    int tableUpperLimit = 300;
    int stepSize = 20;
    print_fahrenheit_celsius_table(stdout, tableLowerLimit, tableUpperLimit, stepSize);
}

double convert_fahrenheit_to_celsius(double fahr)
{
    double celsius = 5 * (fahr - 32) / 9;
    return celsius;
}

void print_fahrenheit_celsius_table(FILE* out, int tableLowerLimit, int tableUpperLimit, int stepSize)
{
    int fahrenheit = tableLowerLimit;
    while (fahrenheit <= tableUpperLimit) {
        int celsius = (int)convert_fahrenheit_to_celsius(fahrenheit);
        fprintf(out, "%d\t%d\n", fahrenheit, celsius);
        fahrenheit += stepSize;
    }
}

I've also changed the conversion function to work with double as seems natural. The table printing code as it stands only wants int, so that is where the truncation should occur. As others noted, other rounding might make more sense.

Note how I've changed the print.. function to take as a parameter the output FILE*. This again makes it the caller's problem to determine where it wants output. Not that important for a beginner program, but a very useful habit down the track in that it allows much better control of where results go.

Answer 3

It depends. Do you want to display the table repeatedly? Then something like

void display_temperature_table(int lower, int upper, int step);

int main(void)
{
    display_temperature_table(0, 300, 20);
}

would make sense.

If you are only displaying the table to test your conversion and don't care about the results, then your original program would make sense.

If you do want to display the table for reasons other than testing your conversion function but only once, then either way can work. I personally have a slight preference for using the function, but that's mainly to make things simpler if you later want to display the table a second time or if you want to do other things as well.

For testing purposes, there are a number of interesting values (and results), e.g. 212 (100), 32 (0), -40 (-40). You may want to test those as a sort of sanity check on your function.

Answer 4

Minor side issue:

Rather than use "rounding" with integer truncation caused by integer division, or using floating point math just to do an integer round, consider a simple int solution.

Round to nearest with int math can simple occur by adding half the denominator, then dividing.

int fahr_to_celsius(int fahr)
{
  // int celsius = 5 * (fahr-32) / 9;
  int numerator = 5 * (fahr-32);
  int denominator = 9;
  numerator += (numerator >= 0) ? denominator/2 : -denominator/2;
  int celsius = numerator/denominator;
  return celsius;
}

Note: The method rounds half-way cases away from 0, but those do not occur here as the denominator is odd.

---

What is really good about your dividing of the task along functional lines allows changes/improvements like this to simply get incorporated without messing up the rest of the code.

Answer 5

Why not use a for loop instead of a while loop to make it more readable? Also, you should return a double in your function, because, for example, 33F should return 0.55C, but instead it returns 0 because it is of type int. Also, fahr is a very weird abbreviation for Fahrenheit. I would rather use f(for short programs like these) or just typing out fahrenheit(for longer programs).

#include <stdio.h>

double f_to_c(int f);

/* print Farhenheit-Celsius table
   for fahr = 0, 20, ..., 300 *;
   use a function for conversion */
int main(void)
{
  int f, c;
  int lower=0, upper=300, step=20;   
  for (f=lower; f <= upper;f+=step) {
    c = f_to_c(f);
    printf("%d\t%d\n", f, c);

    }
}

double f_to_celsius(int f)
{
  double c = 5 * (f-32) / 9;
  return c;
}