patterncppModerate
Kelvin-Fahrenheit-Celsius temperature converter
Viewed 0 times
temperaturecelsiusfahrenheitkelvinconverter
Problem
I am curious if it would ever be in good practice to omit constructor definitions from a class. Here, the intention of the Temperature class is to simply convert between one temperature scale to another. Since there are three scale measurements, kelvin, Fahrenheit, and Celsius, I decided it was best to define specific functions for the desired conversion. All of the functions take a single argument of type double. This leaves me to think it's best to delete the constructors I defined. Also, any other feedback is welcomed too.
```
/*
Create a Temperature class that returns a conversion to a user
selected scale
Kelvin = Celsius + 273.15
Celsius = (5.0/9) * (Fahrenheit - 32)
*/
#include
class Temperature
{
public:
//default constructor
Temperature() : kelvin(NULL), fahrenheit(NULL), celsius(NULL)
{/body intentionally left blank/}
//constructor: Kelvin, Fahrenheit, Celsius
Temperature(double kel, double fahr, double cel)
: kelvin(kel), fahrenheit(fahr), celsius(cel)
{/body intentionally left blank/}
double kelToCel(double);
double kelToFahr(double);
double celToKel(double);
double celToFahr(double);
double fahrToCel(double);
double fahrToKel(double);
private:
double kelvin = 0;
double fahrenheit = 0;
double celsius = 0;
};
double Temperature::kelToCel(double kel)
{
kelvin = kel;
return celsius = kel - 273.15;
}
double Temperature::kelToFahr(double kel)
{
kelvin = kel;
return fahrenheit = (9.0 / 5) * (kel - 273.15) + 32;
}
double Temperature::celToKel(double cel)
{
celsius = cel;
return kelvin = cel + 273.15;
}
double Temperature::celToFahr(double cel)
{
celsius = cel;
return fahrenheit = cel * (9.0 / 5) + 32;
}
double Temperature::fahrToCel(double fahr)
{
fahrenheit = fahr;
return celsius = (5.0 / 9) * (fahr - 32);
}
double Temperature::fahrToKel(
```
/*
Create a Temperature class that returns a conversion to a user
selected scale
Kelvin = Celsius + 273.15
Celsius = (5.0/9) * (Fahrenheit - 32)
*/
#include
class Temperature
{
public:
//default constructor
Temperature() : kelvin(NULL), fahrenheit(NULL), celsius(NULL)
{/body intentionally left blank/}
//constructor: Kelvin, Fahrenheit, Celsius
Temperature(double kel, double fahr, double cel)
: kelvin(kel), fahrenheit(fahr), celsius(cel)
{/body intentionally left blank/}
double kelToCel(double);
double kelToFahr(double);
double celToKel(double);
double celToFahr(double);
double fahrToCel(double);
double fahrToKel(double);
private:
double kelvin = 0;
double fahrenheit = 0;
double celsius = 0;
};
double Temperature::kelToCel(double kel)
{
kelvin = kel;
return celsius = kel - 273.15;
}
double Temperature::kelToFahr(double kel)
{
kelvin = kel;
return fahrenheit = (9.0 / 5) * (kel - 273.15) + 32;
}
double Temperature::celToKel(double cel)
{
celsius = cel;
return kelvin = cel + 273.15;
}
double Temperature::celToFahr(double cel)
{
celsius = cel;
return fahrenheit = cel * (9.0 / 5) + 32;
}
double Temperature::fahrToCel(double fahr)
{
fahrenheit = fahr;
return celsius = (5.0 / 9) * (fahr - 32);
}
double Temperature::fahrToKel(
Solution
Rule of zero
Addressing your first question about destructors: the general rule to follow is that you hardly ever need to hand-code copy/move constructors, copy/move assignment operators and destructors, unless your class manages resources. That's called the Rule of Zero and it helps to easily write classes. Basically, there are two kinds of classes: the ones that manage resources and only do that (a class shouldn't try to do too many things, it should focus on doing one thing, that's the single responsibility principle) and the ones that don't. Most of the classes we write don't manage resources and in this case, it's better to let the compiler generate the special functions.
Design
I have the feeling that your class will be hard to use and maintain in a project: you are storing three different representations for a temperature and the user has to know which was used before using any the methods. That's some strong cognitive burden for users.
Ideally, you should have had a class
For example, you could always store Kelvin degrees and let the functions do the conversions (
Miscellaneous C++ things
-
Your default constructor uses
-
That said, you could simply drop the default constructor. Actually, use
Since you have in-class initializers, the explicitly defaulted default constructor will use them to initialize the class members.
-
Your
Addressing your first question about destructors: the general rule to follow is that you hardly ever need to hand-code copy/move constructors, copy/move assignment operators and destructors, unless your class manages resources. That's called the Rule of Zero and it helps to easily write classes. Basically, there are two kinds of classes: the ones that manage resources and only do that (a class shouldn't try to do too many things, it should focus on doing one thing, that's the single responsibility principle) and the ones that don't. Most of the classes we write don't manage resources and in this case, it's better to let the compiler generate the special functions.
Design
I have the feeling that your class will be hard to use and maintain in a project: you are storing three different representations for a temperature and the user has to know which was used before using any the methods. That's some strong cognitive burden for users.
Ideally, you should have had a class
Temperature which does not expose its representation. You should be able to tell to the class which degrees you are storing and which you want to retrieve. Basically, it would be something like that:class Temperature
{
public:
void set_kelvin(double val);
void set_celsius(double val);
void set_fahrenheit(double val);
double as_kelvin() const;
double as_celsius() const;
double as_fahrenheit() const;
private:
// Here, *you* choose how you store the temperature,
// the user does not have to know about it
double _degrees;
};For example, you could always store Kelvin degrees and let the functions do the conversions (
set_kelvin and as_kelvin wouldn't do more than assigning and returning the value). The user does not have to know how it is stored and what they have stored, only how to set and get the temperature the way they want.Miscellaneous C++ things
-
Your default constructor uses
NULL. Since you use in-class initializers, I guess that you at least use C++11. Therefore, you should use nullptr instead of NULL when you can. However, in your case, the use of NULL/nullptr is wrong: you're not initializing pointers anywhere, only double values. So what is happening is that NULL is interpreted as 0 then converted to the double value 0.0. What you should have done is use 0.0 directly, which is the correct value of the correct type.-
That said, you could simply drop the default constructor. Actually, use
= default to explicitly the compiler to generate it for you sinnce it you don't explicitly ask for it, it will be implicitly deleted because of the presence of other user-defined constructors:Temperature() = default;Since you have in-class initializers, the explicitly defaulted default constructor will use them to initialize the class members.
-
Your
main function contains an unused count variable that you could delete. Your compiler should have warned you about it if you compile with a sufficient level of warnings.Code Snippets
class Temperature
{
public:
void set_kelvin(double val);
void set_celsius(double val);
void set_fahrenheit(double val);
double as_kelvin() const;
double as_celsius() const;
double as_fahrenheit() const;
private:
// Here, *you* choose how you store the temperature,
// the user does not have to know about it
double _degrees;
};Temperature() = default;Context
StackExchange Code Review Q#91627, answer score: 11
Revisions (0)
No revisions yet.