Temperature conversion table

Question

This is another solution for this challenge.

Problem statement:

In this challenge, write a program that takes in three arguments, a start temperature (in Celsius), an end temperature (in Celsius) and a step size. Print out a table that goes from the start temperature to the end temperature, in steps of the step size; you do not actually need to print the final end temperature if the step size does not exactly match. You should perform input validation: do not accept start temperatures less than a lower limit (which your code should specify as a constant) or higher than an upper limit (which your code should also specify). You should not allow a step size greater than the difference in temperatures.

I want to learn more about C++, so if there is some cool C++ feature I could should have used, please comment (or include it in your answer).

#import <iostream>
#import <cmath>

#define COLUMN_SEPARATOR "\t| "
#define MAX_TEMP 500
#define MIN_TEMP -500

inline bool between(double x, double max, double min) {
    return max >= x && min <= x;
}

void getInput(double &lower, double &upper, double &step) {
    double temp1, temp2;
    std::cout << "Please enter consecutively the upper and lower limits, both between " <<  MIN_TEMP << " and " << MAX_TEMP << "." << std::endl;
    std::cin >> temp1;
    std::cin >> temp2;
    while (!between(temp1, MAX_TEMP, MIN_TEMP) || !between(temp2, MAX_TEMP, MIN_TEMP)) {
        std::cout << "At least one of the temperatures is out of bounds. Please reenter:" << std::endl;
        std::cin >> temp1;
        std::cin >> temp2;
    }
    upper = std::max(temp1, temp2);
    lower = std::min(temp1, temp2);
    std::cout << "Please enter a positive stepsize, smaller than the difference between the limits." << std::endl;
    std::cin >> step;
    while (step < 0 || step > upper - lower) {
        std::cout << "The stepsize is out of bounds. Please reenter:" << std::endl;
        std::cin >> step;
    }
}

double toFahrenheit(double celsius) {
    return celsius*(9/5) + 32;
}

void printTable(double start, double end, double step) {
    std::cout << "Celsius" << COLUMN_SEPARATOR << "Fahrenheit" << std::endl;
    std::cout << "=======" << COLUMN_SEPARATOR << "==========" << std::endl;
    for (double i = start; i < end; i += step) {
        std::cout << i << COLUMN_SEPARATOR << toFahrenheit(i) << std::endl;
    }
}

int main() {
    double start, end, step;
    getInput(start, end, step);
    printTable(start, end, step);
    return 0;
}

Sample run:

192:Challenges 11684$ ./a.out Please enter consecutively the upper and lower limits, both between -500 and 500.
3.692
65.937
Please enter a positive stepsize, smaller than the difference between the upper and lower limit.
5.3729
Celsius   | Fahrenheit
=======   | ==========
3.692 | 35.692
9.0649    | 41.0649
14.4378   | 46.4378
19.8107   | 51.8107
25.1836   | 57.1836
30.5565   | 62.5565
35.9294   | 67.9294
41.3023   | 73.3023
46.6752   | 78.6752
52.0481   | 84.0481
57.421    | 89.421
62.7939   | 94.7939

Answer 1

You don't check for invalid input.

std::cin >> temp1;
std::cin >> temp2;

What if I type BLA BLA<enter> on the input?

As user input is line based. Most programers decide to get a single value at a time.

std::cout << "Please enter consecutively the upper and lower limits, both between " <<  MIN_TEMP << " and " << MAX_TEMP << "." << std::endl;

Now your technique is not wrong. But you definitely make it harder for your self to validate the input and user interaction is not that great as they are used to typing one value return (and getting feedback on that value).

I would change that getInput() so that each value is queried for separately (and use a function to get the value).

    lower  = getUserInput("Please Enter the lower limit of the table", [](int x){return x >= MIN_TEMP;});
    upper  = getUserInput("Please Enter the upper limit of the table", [](int x){return x <= MAX_TEMP;});
    step   = getUserInput("Please Enter the step size",                [](int){return true;});

Don't use macros for constants.

#define MAX_TEMP 500
#define MIN_TEMP -500

That's really old school C. Macros have no concept of scope or type. As such they can potentially clash with other people's macros. Prefer to use const values.

static const int maxTemp = 500;
static const int minTemp = -500;

Or if you are using C++11 and above.

static constexpr int maxTemp = 500;
static constexpr int minTemp = -500;

As a physics persons. You may find that -500 is too low a value (you can not cool things to that temperature ( 0 Kelvin is the lowest temperature theoretically (though there have been experiments that show a temperature a few fractions below this but that has more to do with how we measure the temperature and people are still arguing about it))).

Answer 2

Looks like there are no Americans here. 30 degrees Celsius is about 90 degrees Fahrenheit, so something must be wrong in the conversion. Indeed, (9/5) is evaluated (at compile time) as an integer division, yielding 1. Change it to (9.0/5.0).

Answer 3

• One fundamental thing is that macros should used as constants:

  #define MAX_TEMP 500
  #define MIN_TEMP -500
  

  should use the const keyword:

  const int max_temp = 500;
  const int min_temp = -500;
  

  Since macros are commonly all-uppercase, these can use camelCase or snake_case.

  You could also put min_temp above max_temp for a bit more readability.

• Keep in mind that the inline keyword is mostly a compiler hint. In other words, the compiler is free to ignore you if it chooses not to take that hint for whatever reason.

  Side-note: I would've expected inline to also be used for toFahrenheit(), since it is also a single-line function. But like I said, the compiler may still leave it out.

• It may be a little more readable to change this:

  while (step < 0 || step > upper - lower)
  

  to this (with more parenthesis):

  while ((step < 0) || (step > upper - lower))
  

  This could make it easier to tell that both sides are being compared with ||.

• Try not to declare/initialize variables on the same line:

  double start, end, step;
  

  If you end up adding additional ones, it'll just make the line longer and longer. Instead, have each one on a separate line:

  double start;
  double end;
  double step;
  

• You don't need to explicitly return 0 at the end of main(). Reaching this point always indicates success termination, so the compiler will do the return here for you.

Answer 4

• Don't use #import. This is not standard C++, and will be treated differently by different compilers. Microsoft uses it to indicate that the included file is a type library, while gcc will pretend the included file has include guards.^† The standard library will always have include guards, so just use #include in this case.

• Your COLUMN_SEPARATOR uses a \t for alignment, which explains the wonky third line. It would be better to use std::setw from <iomanip> here:

  std::cout << std::setw(10) << i << COLUMN_SEPARATOR << std::setw(10) << toFahrenheit(i) << std::endl;
  

  If you're feeling ambitious, you can try overloading the << operator, as suggested in this SO answer.

---

^{† That is to say, it will include the file at most once.}