3
\$\begingroup\$

Problem statement:

Write a program that converts all given temperatures from a given input temperature scale to a given output temperature scale. The temperature scales to be supported are Kelvin, Celsius, Fahrenheit, Rankine, Delisle, Newton, Rømer, Réaumur.

Synopsis: tempconv INPUT_SCALE OUTPUT_SCALE [TEMPERATURE]...

The INPUT_SCALE and OUTPUT_SCALE shall be given as follows:

  • K for Kelvin
  • C for Celsius
  • F for Fahrenheit
  • R for Rankine
  • D for Delisle
  • N for Newton
  • for Rømer
  • for Réaumur.

Example:

tempconv K C 0 273.15 373.15
-273.15
0.0
100.0

My solution in Python:

#!/usr/bin/python3

import sys
from typing import Callable
from dataclasses import dataclass


@dataclass(frozen=True)
class TemperatureConverter:
    to_kelvin: Callable[[float], float]
    from_kelvin: Callable[[float], float]
    names: list[str]


class TemperatureConverters:
    CELSIUS    = TemperatureConverter(lambda celsius:    celsius + 273.15,                 lambda kelvin: kelvin - 273.15,                   ["°C", "C", "c"])
    KELVIN     = TemperatureConverter(lambda kelvin:     kelvin,                           lambda kelvin: kelvin,                            ["K", "k"])
    FAHRENHEIT = TemperatureConverter(lambda fahrenheit: (fahrenheit + 459.67) * 5 / 9,    lambda kelvin: kelvin * 9 / 5 - 459.67,           ["°F", "F", "f"]),
    RANKINE    = TemperatureConverter(lambda rankine:    rankine * 5 / 9,                  lambda kelvin: kelvin * 9 / 5,                    ["°R", "R", "r"]),
    DELISLE    = TemperatureConverter(lambda delisle:    373.15 - delisle * 2 / 3,         lambda kelvin: (373.15 - kelvin) * 3 / 2,         ["°De", "De", "DE", "de"]),
    NEWTON     = TemperatureConverter(lambda newton:     newton * 100 / 33 + 273.15,       lambda kelvin: (kelvin - 273.15) * 33 / 100,      ["°N", "N", "n"]),
    RÉAUMUR    = TemperatureConverter(lambda réaumur:    réaumur * 5 / 4 + 273.15,         lambda kelvin: (kelvin - 273.15) * 4 / 5,         ["°Ré", "°Re", "Ré", "RÉ", "ré", "Re", "RE", "re"]),
    RØMER      = TemperatureConverter(lambda rømer:      (rømer - 7.5) * 40 / 21 + 273.15, lambda kelvin: (kelvin - 273.15) * 21 / 40 + 7.5, ["°Rø", "°Ro", "Rø", "RØ", "rø", "Ro", "RO", "ro"]),

    @classmethod
    def get(cls, name: str):
        for prop, val in vars(cls).items():
            if isinstance(val, TemperatureConverter):
                if name in val.names:
                    return val


inputConverter = TemperatureConverters.get(sys.argv[1]).to_kelvin
outputConverter = TemperatureConverters.get(sys.argv[2]).from_kelvin
for arg in sys.argv[3:]:
    print(outputConverter(inputConverter(float(arg))))

I know that the lines defining the conversions are a bit long, I'm okay with sacrificing line length to retain a readable tabular format in such a case.

I'm looking for more concise ways, for example, is there a way to turn the @classmethod get() into an operator, like in Kotlin? I tried to change it to __getitem__, but Python didn't like that for a @classmethod. Also, is there a smart way to join TemperatureConverter and TemperatureConverters into one class? I tried, and I found that difficult to do without losing the conciseness from @dataclass(frozen=True).

\$\endgroup\$

2 Answers 2

6
\$\begingroup\$

Use argparse.

Note that all of your conversions are linear in one dimension. That means that you don't need two lambdas per conversion: just store a coefficient and offset from Kelvin for each temperature unit.

names should be a tuple.

Instead of dataclass, NamedTuple is a simpler alternative that is still frozen.

I should mention, I like that you've taken function bindings from your converter instances prior to the loop.

Suggested

Only some of the conversions shown, for demonstration:

#!/usr/bin/python3

import argparse
from typing import NamedTuple


class TemperatureConverter(NamedTuple):
    offset: float
    coefficient: float
    names: tuple[str, ...]

    def to_kelvin(self, temp: float) -> float:
        return self.coefficient*temp + self.offset

    def from_kelvin(self, temp: float) -> float:
        return (temp - self.offset)/self.coefficient


CELSIUS    = TemperatureConverter(  273.15,    1, ("°c", "c"))
KELVIN     = TemperatureConverter(    0.00,    1, ("k",))
FAHRENHEIT = TemperatureConverter(45967/180, 5/9, ("°f", "f"))

CONVERTERS = {
    name: converter
    for converter in (CELSIUS, KELVIN, FAHRENHEIT)
    for name in converter.names
}


def main() -> None:
    parser = argparse.ArgumentParser()
    parser.add_argument('input_unit')
    parser.add_argument('output_unit')
    parser.add_argument('temperatures', type=float, nargs='+')
    args = parser.parse_args()

    convert_input = CONVERTERS[args.input_unit.lower()].to_kelvin
    convert_output = CONVERTERS[args.output_unit.lower()].from_kelvin

    for temperature in args.temperatures:
        output_temp = convert_output(convert_input(temperature))
        print(f'{output_temp:.2f}')


if __name__ == '__main__':
    main()
\$\endgroup\$
4
\$\begingroup\$

Following the open-close principle

That is, designing classes to be open to extension and closed to modifications.

I have some concerns about the current code:

  • If I wanted to add support for another temperature converter, I will have to modify the implementation of the TemperatureConverters class
  • ... and be careful to not use a symbol that has already been used by something else
  • To review this code is correct, I have to manually check that the same symbol is not used twice

I think a simple alternative would be if TemperatureConverters was more of a regular class, with a registerConverter method, which would validate that the new converter is not using a symbol that's already taken.

It will also make sense to keep the mapping of symbol to converter in a dictionary for fast lookups. That will be a good improvement on the current approach, which has to loop over converter objects and their possible symbols.

Maybe this alternative will not be as concise as the original with tabular writing style, but it will be more convincingly correct, and also extensible without modifying existing code.

\$\endgroup\$
2
  • \$\begingroup\$ I know that this is the definition of Bertrand Meyer's OCP that Robert C. Martin usually gives. I disagree with this definition because it makes it sound that the problem is specific to object-oriented programming with classes. The real problem that the OCP addresses is that adding something in one place will require making changes in other places as well. In that spirit, the code follows the OCP already. My definition: "It should be possible to add behavior by adding code rather than modifying existing code." \$\endgroup\$ Commented Sep 7, 2022 at 13:14
  • \$\begingroup\$ P.S.: Still upvoted. Yes, a dict. Changing the solution to use a dict right now. (Already using a map in most other languages, like Java, Kotlin, C++.) \$\endgroup\$ Commented Sep 7, 2022 at 13:16

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.