Capturing temperature information for a patient

I just finished writing a program that captures temperature information for a patient. I'm looking for suggestions on how to improve the code that I have. Although what I have compiles and does what I want, I wonder if there is any methodology I'm using inappropriately despite it still working.

Some particular areas I worry about are redundant code and my use of the conversion and fever methods (you can ignore the clearly bad math. I just wanted any calculation until I determined functionality).

Temperature.hpp:

#ifndef Temperature_h
#define Temperature_h

#include 
#include 
using namespace std;

class Temperature {
private:
    string newPatient;
    float newTemp;
    char newDegree;

public:
    enum TempMethod {oral, arm, leg, bum};

    Temperature();
    Temperature(string, float, char);
    ~Temperature();

    string getPatient() const;
    float getTemp() const;
    char getDegree() const;

    float setFaren();
    float setCels();
    void hasFever();

};

#endif /* Temperature_h */

Temperature.cpp:

Temperature::Temperature(string patient, float temp, char degree){
    newPatient = patient;
    newTemp = temp;
    newDegree = degree;
}

Temperature::~Temperature(){

}

string Temperature::getPatient() const{
    return newPatient;
}

float Temperature::getTemp() const{
    return newTemp;
}

char Temperature::getDegree() const{
    return newDegree;
}

float Temperature::setFaren(){
    float faren = newTemp+32;
    return faren;
}

float Temperature::setCels(){
    float celsi = newTemp-32;
    return celsi;
}

void Temperature::hasFever(){
    if(newTemp >= 100.4){
        cout << "Patient " << newPatient << " has a fever!!\n";
    }
}

main.cpp:

```
#include
#include
#include "Temperature.h"
using namespace std;

int main() {
string patient;
float temp;
float celsius;
float farenheit;
char degree;

cout > temp;
cout > degree;

Temperature Temp_1(patient, temp, degree);
cout << "\nPatient Name

Technicalities

Your destructor is the same as the default destructor -- I wouldn't bother defining one. It makes me think that the class is going to actually do something in there, so it's strange when it doesn't.

Your header doesn't use iostream, only your implementation does. This means that the #include should be in the cpp file. Try to keep headers as tightly included as possible.

You're not mutating the patient variable so there's no need to make a copy. Instead, you should take it by const reference.

Temperature(const std::string& patient, float temp, char degree);

Instead of assigning your parameters, you should use an initializer list. Assinging means that they might be default constructed first then assigned, and this is unnecessary.

Temperature::Temperature(const std::string& patient, float temp, char degree)
  : patient(patient), temp(temp), degree(degree)
{ }

You did a good job of making your getters const, but getFahrenheit, getCelsius and hasFever should all also be const (they should also be renamed as this implies -- see the Style section).

You shouldn't use using namespace std in header files.

For little toy programs like this, it doesn't matter much, but it can be a good habit to always remember that any kind of IO can fail. In particular, if you want to indulge in some paranoia, you should check that your cin extraction was successful. After all, what if someone puts in a non-sense value when you're expecting a float. You don't particularly want your program to continue after that.

Style

float Temperature::setFaren(){
    float faren = newTemp+32;
    return faren;
}

This isn't setting anything. This is a getter. It should be named getFaren, or even better, getFahrenheit. To get a bit cliche-y for a second, code is read much more than it's written. Strive for clarity even if that means typing a few extra characters.

Also, the variable is a bit superfluous. I'd go with a simple return newTemp + 32;.

#ifndef Temperature_h

Include guards are traditionally SCREAMING_SNAKE_CASE:

#ifndef TEMPERATURE_H

Temperature(string, float, char);

This is confusing as a consumer of the class. You should give these parameters names:

Temperature(string patient, float temperature, char degree);

You have some non-optimal namings. For example, newPatient, newTemp, and newDegree don't really make sense. Why are they new? Just call them patient, temp and degree. If you're worried about them clashing with the local variables in the constructor, you can just use this->patient. I would also consider calling patient patientName since patient makes me expect some kind of model rather than a simple string.

I also am not a fan of the parameter name temp. That makes me assume it's some kind of temporary variable. temperature is clearer.

degree also unfortunately has issues. Fahrenheit and Celsius aren't degrees. They're scales of temperatures. This parameter could instead be named scale.

void Temperature::hasFever(){
    if(newTemp >= 100.4){
        cout << "Patient " << newPatient << " has a fever!!\n";
    }
}

This shouldn't output, but should instead return a bool. The consumer of the class should decide what to do with this information. What if you had a list of Temperatures and you wanted to count how many instances were considered to have a fever. You would likely not want this to output each time a fever was found. Instead, you want the method to return a bool that you can then decide what you want to do with it.

Also, you need to be aware of what scale you're using here. The threshold for a fever in Celsius is much different than the threshold in Fahrenheit.

enum TempMethod {oral, arm, leg, bum};

As this seems to be unused, I would remove it.

If this code were going to live longer than 1 homework assignment or practice, I would expect the class and its methods to be documented.

Design

I would decide on a canonical scale and use that internally. Then, your other scale(s) would simply convert in their getters. For example, you could always store Celsius and just convert to Fahrenheit when needed (this would of course mean that your constructor would need to convert to Celcius when Fahrenheit is provided).

I would consider using an enum for your scale instead of a char. An enum would mean that a user could only provide correct values. It also more explicitly signals what is supported. For example, someone might try to pass in K as your code currently is, then your conversion methods might not support that. Even worse, someone could currently pass in some non-sense like a.

If you do keep it a char, make sure to validate that it's one of the expected values.

It's strange for Temperature to have a patient name. It means that you're not really modeling a temperature; you're modeling a patient/temperature combination. In other words, you've coupled y

StackExchange Code Review Q#126078, answer score: 7