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

• 200_success: codereview.stackexchange.com/q/44836/9357
• Jamal: How did this work with #import? It's supposed to be #include.
• Schism: @Jamal In short, #import is a non-portable #include.
• 11684: @Jamal Oops, too much Objective-C. But I swear it compiles with g++ -Wall tempConverter.cpp. Without warnings/errors/compiler complaints.
• PlasmaHH: @11684: That entirely depends on your project. What I can recommend though is to take the time and read through all warning options in your manpage. Those are also intresting concerning typical mistakes you could make. Just don't use -Weffc++

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))).

• 11684: So getUserInput() uses the lambda to check if the input is valid?
• Loki Astari: @11684: It was just a suggestion. I am sure you can think of any number of other ways to do it.
• 11684: Of course, but that is way more elegant than what I had, plus it means using a cool C++ feature (lambdas), which is why I did this "challenge".

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).

• Loki Astari: Whether it is done at compile time or run time is an implementation detail. But it is (as you correctly pointed out) integer division; which results in an integer. I should have seen that. Good eyes.
• Aurelius: Or simply omit the parentheses and let operator precedence do its thing. But changing the literals to the correct type is probably a better solution.

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.

• 11684: Should temp1 and temp2 be on their own lines too?
• Jamal: @11684: They could, though it's not that crucial since they're very similar and there can only be two of them. My comment on that was primarily a general one (when you have more variables).

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.}