patterncppModerate
Celsius → Fahrenheit conversion table
Viewed 0 times
fahrenheitconversioncelsiustable
Problem
It's harder to do than criticize. Here's my attempt to implement a Celsius-to-Fahrenheit conversion table in C++.
Some concerns I have:
#include
#include
#include
#include
#define DEGREE_SIGN "\u00B0"
class Fahrenheit;
class Celsius {
public:
explicit Celsius(double c) : c(c) {
if (c MAX_VALUE) {
throw "Temperature above maximum";
}
}
Celsius(Fahrenheit f);
operator double() const { return c; }
Celsius operator +(Celsius incr) const { return Celsius(c + incr.c); }
Celsius &operator +=(Celsius incr) { c += incr.c; return *this; }
friend std::ostream &operator MAX_VALUE) {
throw "Temperature above maximum";
}
}
Fahrenheit(Celsius c);
operator double() const { return f; }
Fahrenheit operator +(Fahrenheit incr) const { return Fahrenheit(f + incr.f); }
Fahrenheit &operator +=(Fahrenheit incr) { f += incr.f; return *this; }
friend std::ostream &operator max - min) {
std::cerr << "Step exceeds temperature range" << std::endl;
return 1;
} else if (max < min) {
std::cerr << "Minimum temperature exceeds maximum temperature" << std::endl;
return 1;
}
for (Celsius c = min; c <= max; c += step) {
std::cout << std::setw(8) << c
<< std::setw(8) << Fahrenheit(c)
<< std::endl;
}
} catch (const char *msg) {
std::cerr << msg << std::endl;
return 1;
}
return 0;
}Some concerns I have:
- What do you think of the input validation?
- Is the code repetition justified? Much of the
Fahrenheitclass is not strictly necessary for the problem, and is included for completeness. Still, does the repetition enhance clarity, or is it harmful?
Solution
- Don't throw string literals directly. Instead throw an instance of
std::exceptionor (even better) a subclass thereof (in this case, I would choose eitherstd::domain_errororstd::out_of_range). It makes it easier for people to catch usingcatch (std::exception&).
- The
Celsius(Fahrenheit)constructor should probably take its argument by const reference.
- I don't like the
operator doubleconversion operator; it seems too easy to accidentally invoke. I think it's better to have a member function that you have to explicitly call to get its numeric value.
-
operator+ is usually better to be a free function. That way, you can use the standard implementation:Celsius operator+(Celsius lhs, Celsius const& rhs) {
return lhs += rhs;
}noting that
lhs is passed by value. There are other reasons for having operator+ be a free function, such as being able to use other types for the LHS, other types for the RHS, etc., but here, the main advantage is that you can take the LHS by value.operator+=should take its argument by const reference.
- I would actually embed the degree sign straight into the string literal(s), rather than using a
#define(the use of which is discouraged in C++). We live in a UTF-8 world.
- Do not use C-style casts (
(double)cas you had it). Here, it's much clearer to just writec.value()(if you added that member function as I suggested above; this has the added benefit of not requiringfriendaccess) orc.c. In the general case of casting, it'd be better to use (in this case) astatic_cast.
Fahrenheit::MIN_VALUEandFahrenheit::MAX_VALUEshould be calculated off the value ofCelsius::MIN_VALUEandCelsius::MAX_VALUE.
- All the comments about
Celsiusabove apply toFahrenheittoo.
- Use
std::cerrto print out the usage message, notstd::cout.
- Use
'\n'instead ofstd::endl. See The littleendlthat couldn't for rationale.
- It's much more robust to parse numbers using
boost::lexical_castinstead ofstrtodand then checking the end pointers. In particular, your code doesn't handle the case of empty strings correctly, and will treat them as valid.
- The
elses in your error checks are superfluous since all of the precedingifbranches are exiting conditions.
- Use
EXIT_SUCCESSandEXIT_FAILUREfor the exit codes instead of 0 and 1. Also, the bottom successfulreturnis superfluous and should be removed.
To answer your bottom question, by YAGNI principles, much of your
Fahrenheit class should be omitted. But if you want to be general, you could have written a templated TemperatureConverter, of which Celsius and Fahrenheit could both be specialised typedefs.I also agree with the other two answers (also by Chrises, hah), and have upvoted them both.
Code Snippets
Celsius operator+(Celsius lhs, Celsius const& rhs) {
return lhs += rhs;
}Context
StackExchange Code Review Q#44836, answer score: 14
Revisions (0)
No revisions yet.