Skip to main content
Code Review

Return to Answer

edited body
Source Link
Konrad Rudolph
  • 6.7k
  • 23
  • 35

Your problem is completely unrelated to smart pointers.

The keyword here is separation of concerns. You have a function (well, several of them) who try to do everything: create a prompt, control user input, convert a value, and output the result.

This is bad. You should separate these concerns rigorously.

At the heart of your program is a conversion function with the prototype

double convert(double input);

Of course, you have several of them – one for each unit-to-unit conversion (this can be done differently by first converting to a common unit but we’ll omit this for now).

Then you need an input and an output routine, which may look as follows:

double read_number(std::string const& unit) {
 std::cout << "Please enter the temperature in " << unit << std::flush;
 double value;
 std::cin >> value;
 return value;
}

and

void print_result(std::string const& from_unit, std::string const& to_unit, double from, double to) {
 std::cout << from << " " << from_unit << " is equivalent to "
 << to << " " << to_unit << "\n";
}

The conversions are then straightforward one-liners, for instance:

double celsius_to_kelvin(double value) {
 return value + 273.15;
}

(Furthermore, note that while it’s "degrees celsius", it’s simply "kelvin" – not "degrees kelvin".)

You could even use the same function for all conversions and merely store the scaling factor and offset. Such as:

typedef std::pair<double, double> coefficients_t;
std::vector<coefficients_t> conversions = {
 { 273.15, 01 }, // °C => K
 { -32, 0.5556 }, // °F => °C
 ...
};
enum conversion_type {
 CtoK,
 FtoC,
 ...
};
double convert(conversion_type conversion, double input) {
 coefficients_t coef = conversions[conversion];
 return (input + coeffs.first) * coeffs.second;
}

(This syntax requires C++11, C++03 is more verbose.)

Your problem is completely unrelated to smart pointers.

The keyword here is separation of concerns. You have a function (well, several of them) who try to do everything: create a prompt, control user input, convert a value, and output the result.

This is bad. You should separate these concerns rigorously.

At the heart of your program is a conversion function with the prototype

double convert(double input);

Of course, you have several of them – one for each unit-to-unit conversion (this can be done differently by first converting to a common unit but we’ll omit this for now).

Then you need an input and an output routine, which may look as follows:

double read_number(std::string const& unit) {
 std::cout << "Please enter the temperature in " << unit << std::flush;
 double value;
 std::cin >> value;
 return value;
}

and

void print_result(std::string const& from_unit, std::string const& to_unit, double from, double to) {
 std::cout << from << " " << from_unit << " is equivalent to "
 << to << " " << to_unit << "\n";
}

The conversions are then straightforward one-liners, for instance:

double celsius_to_kelvin(double value) {
 return value + 273.15;
}

(Furthermore, note that while it’s "degrees celsius", it’s simply "kelvin" – not "degrees kelvin".)

You could even use the same function for all conversions and merely store the scaling factor and offset. Such as:

typedef std::pair<double, double> coefficients_t;
std::vector<coefficients_t> conversions = {
 { 273.15, 0 }, // °C => K
 { -32, 0.5556 }, // °F => °C
 ...
};
enum conversion_type {
 CtoK,
 FtoC,
 ...
};
double convert(conversion_type conversion, double input) {
 coefficients_t coef = conversions[conversion];
 return (input + coeffs.first) * coeffs.second;
}

(This syntax requires C++11, C++03 is more verbose.)

Your problem is completely unrelated to smart pointers.

The keyword here is separation of concerns. You have a function (well, several of them) who try to do everything: create a prompt, control user input, convert a value, and output the result.

This is bad. You should separate these concerns rigorously.

At the heart of your program is a conversion function with the prototype

double convert(double input);

Of course, you have several of them – one for each unit-to-unit conversion (this can be done differently by first converting to a common unit but we’ll omit this for now).

Then you need an input and an output routine, which may look as follows:

double read_number(std::string const& unit) {
 std::cout << "Please enter the temperature in " << unit << std::flush;
 double value;
 std::cin >> value;
 return value;
}

and

void print_result(std::string const& from_unit, std::string const& to_unit, double from, double to) {
 std::cout << from << " " << from_unit << " is equivalent to "
 << to << " " << to_unit << "\n";
}

The conversions are then straightforward one-liners, for instance:

double celsius_to_kelvin(double value) {
 return value + 273.15;
}

(Furthermore, note that while it’s "degrees celsius", it’s simply "kelvin" – not "degrees kelvin".)

You could even use the same function for all conversions and merely store the scaling factor and offset. Such as:

typedef std::pair<double, double> coefficients_t;
std::vector<coefficients_t> conversions = {
 { 273.15, 1 }, // °C => K
 { -32, 0.5556 }, // °F => °C
 ...
};
enum conversion_type {
 CtoK,
 FtoC,
 ...
};
double convert(conversion_type conversion, double input) {
 coefficients_t coef = conversions[conversion];
 return (input + coeffs.first) * coeffs.second;
}

(This syntax requires C++11, C++03 is more verbose.)

added 746 characters in body
Source Link
Konrad Rudolph
  • 6.7k
  • 23
  • 35

Your problem is completely unrelated to smart pointers.

The keyword here is separation of concerns. You have a function (well, several of them) who try to do everything: create a prompt, control user input, convert a value, and output the result.

This is bad. You should separate these concerns rigorously.

At the heart of your program is a conversion function with the prototype

double convert(double input);

Of course, you have several of them – one for each unit-to-unit conversion (this can be done differently by first converting to a common unit but we’ll omit this for now).

Then you need an input and an output routine, which may look as follows:

double read_number(std::string const& unit) {
 std::cout << "Please enter the temperature in " << unit << std::flush;
 double value;
 std::cin >> value;
 return value;
}

and

void print_result(std::string const& from_unit, std::string const& to_unit, double from, double to) {
 std::cout << from << " " << from_unit << " is equivalent to "
 << to << " " << to_unit << "\n";
}

The conversions are then straightforward one-liners, for instance:

double celsius_to_kelvin(double value) {
 return value + 273.15;
}

(Furthermore, note that while it’s "degrees celsius", it’s simply "kelvin" – not "degrees kelvin".)

You could even use the same function for all conversions and merely store the scaling factor and offset. Such as:

typedef std::pair<double, double> coefficients_t;
std::vector<coefficients_t> conversions = {
 { 273.15, 0 }, // °C => K
 { -32, 0.5556 }, // °F => °C
 ...
};
enum conversion_type {
 CtoK,
 FtoC,
 ...
};
double convert(conversion_type conversion, double input) {
 coefficients_t coef = conversions[conversion];
 return (input + coeffs.first) * coeffs.second;
}

(This syntax requires C++11, C++03 is more verbose.)

Your problem is completely unrelated to smart pointers.

The keyword here is separation of concerns. You have a function (well, several of them) who try to do everything: create a prompt, control user input, convert a value, and output the result.

This is bad. You should separate these concerns rigorously.

At the heart of your program is a conversion function with the prototype

double convert(double input);

Of course, you have several of them – one for each unit-to-unit conversion (this can be done differently by first converting to a common unit but we’ll omit this for now).

Then you need an input and an output routine, which may look as follows:

double read_number(std::string const& unit) {
 std::cout << "Please enter the temperature in " << unit << std::flush;
 double value;
 std::cin >> value;
 return value;
}

and

void print_result(std::string const& from_unit, std::string const& to_unit, double from, double to) {
 std::cout << from << " " << from_unit << " is equivalent to "
 << to << " " << to_unit << "\n";
}

Your problem is completely unrelated to smart pointers.

The keyword here is separation of concerns. You have a function (well, several of them) who try to do everything: create a prompt, control user input, convert a value, and output the result.

This is bad. You should separate these concerns rigorously.

At the heart of your program is a conversion function with the prototype

double convert(double input);

Of course, you have several of them – one for each unit-to-unit conversion (this can be done differently by first converting to a common unit but we’ll omit this for now).

Then you need an input and an output routine, which may look as follows:

double read_number(std::string const& unit) {
 std::cout << "Please enter the temperature in " << unit << std::flush;
 double value;
 std::cin >> value;
 return value;
}

and

void print_result(std::string const& from_unit, std::string const& to_unit, double from, double to) {
 std::cout << from << " " << from_unit << " is equivalent to "
 << to << " " << to_unit << "\n";
}

The conversions are then straightforward one-liners, for instance:

double celsius_to_kelvin(double value) {
 return value + 273.15;
}

(Furthermore, note that while it’s "degrees celsius", it’s simply "kelvin" – not "degrees kelvin".)

You could even use the same function for all conversions and merely store the scaling factor and offset. Such as:

typedef std::pair<double, double> coefficients_t;
std::vector<coefficients_t> conversions = {
 { 273.15, 0 }, // °C => K
 { -32, 0.5556 }, // °F => °C
 ...
};
enum conversion_type {
 CtoK,
 FtoC,
 ...
};
double convert(conversion_type conversion, double input) {
 coefficients_t coef = conversions[conversion];
 return (input + coeffs.first) * coeffs.second;
}

(This syntax requires C++11, C++03 is more verbose.)

Source Link
Konrad Rudolph
  • 6.7k
  • 23
  • 35

Your problem is completely unrelated to smart pointers.

The keyword here is separation of concerns. You have a function (well, several of them) who try to do everything: create a prompt, control user input, convert a value, and output the result.

This is bad. You should separate these concerns rigorously.

At the heart of your program is a conversion function with the prototype

double convert(double input);

Of course, you have several of them – one for each unit-to-unit conversion (this can be done differently by first converting to a common unit but we’ll omit this for now).

Then you need an input and an output routine, which may look as follows:

double read_number(std::string const& unit) {
 std::cout << "Please enter the temperature in " << unit << std::flush;
 double value;
 std::cin >> value;
 return value;
}

and

void print_result(std::string const& from_unit, std::string const& to_unit, double from, double to) {
 std::cout << from << " " << from_unit << " is equivalent to "
 << to << " " << to_unit << "\n";
}
lang-cpp

AltStyle によって変換されたページ (->オリジナル) /