HiveBrain v1.2.0
Get Started
← Back to all entries
principlepythonMajor

Comparing Old vs. New: Temperature Converter

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
temperaturenewcomparingoldconverter

Problem

I made this temperature converter when I first started getting into Python. After a year of regular programming on my own I decide it was time to revisit my old program. Now that I finished my first semester of computer science I took my old code and made it more efficient.

New:

```
# Declares the variables need for the converter function.
def declare_var():
# Prompt user to select the initial temperature type.
print("Enter a number to declare the initial temperature type?")
print("[1] Celsius [2] Fahrenheit [3] Kelvin [4] Rankine\n"
"[5] Delisle [6] Newton [7] Reaumer [8] Romer")
initial_temp = int(eval(input('')))

# Select the conversion equation within the list selected.
print("Enter a number to specify the temperature for conversion?")
print("[1] Celsius [2] Fahrenheit [3] Kelvin [4] Rankine\n"
"[5] Delisle [6] Newton [7] Reaumer [8] Romer")
conversion_temp = int(eval(input('')))

# Gets input of temperature degrees.
temp_degree = eval(input('''Enter the temperature degrees\n'''))

return initial_temp, conversion_temp, temp_degree

# Calculates the temperature conversion based on
# returned variables from the declare_vars function.
def converter(function):
# Assigned returned values variable names for readability.
initial_temp = function[0]
conversion_temp = function[1]
temp = function[2]

# The order of the conversion equations go in this order:
# Celsius, Fahrenheit, Kelvin, Rankine, Delisle, Newton, Réaumur, Rømer.
temp_equations = {"Celsius": [temp, # Celsius
temp * 9.0 / 5.0 + 32.0, # Fahrenheit
temp + 273.15, # Kelvin
temp + 273.15 * 9.0 / 5.0, # Rankine
(100 - temp) * 3.0 / 2.0, # Delisle
temp * 33.0 / 100.0, # Newton
temp * 4.0 / 5.0, # Réaumur

Solution

Your second solution is basically a brute-force if-else tree, with lots of repetitive code, and not particularly interesting to review. I'll just focus on the first solution.

Your terminology is confusing, as exemplified by this line near the end of converter():

# Selects the calculated equation for the selected conversion
temp_conversion = (select_equation[conversion_temp - 1])


What does temp_conversion mean? What does conversion_temp mean?

I recommend changing the terminology this way:

  • initial_tempfrom_scale



  • conversion_tempto_scale



  • tempfrom_temp



  • temp_conversiontemp or result



The way converter() accepts its parameters is rather weird:

def converter(function):
    # Assigned returned values variable names for readability.
    initial_temp = function[0]
    conversion_temp = function[1]
    temp = function[2]


What function are you talking about? A more conventional way to write that would be

def converter(from_scale, to_scale, from_temp):
    …


… which you can call using a "splat":

def main():
    converter(*declare_var())


I like that you made a declare_var() function as the input routine. However, that leaves converter() to do the calculation and print the output. A better division of labour would be to have a function that does just the calculation, and a second function to serve as the input/output front-end. If you were to develop a GUI, then the calculation function would be reusable, and you would replace the input/output front-end.

You have a combinatorial problem: with 8 temperature scales, you need to define 8 × 8 = 64 conversion formulas. Not only do you have a lot of formulas, it's also hard to maintain them all correctly. I hope that your # Celsius, # Fahrenheit, # Kelvin, # Rankine, # Delisle, # Réaumur, and # Rømer comments are all in the right place!

Note that when you write…

temp_equations = {"Celsius": [temp,  # Celsius
                              temp * 9.0 / 5.0 + 32.0,  # Fahrenheit
                              temp + 273.15,  # Kelvin
                              temp + 273.15 * 9.0 / 5.0,  # Rankine
                              (100 - temp) * 3.0 / 2.0,  # Delisle
                              temp * 33.0 / 100.0,  # Newton
                              temp * 4.0 / 5.0,  # Réaumur
                              temp * 21.0 / 40.0 + 7.5],  # Rømer


… you aren't defining eight equations. You are actually performing eight conversions. In other words, your converter performs 64 conversions, and discards the result of 63 of those calculations. If you want to define formulas rather than values, then they need to be functions — ideally written using lambda.

A better design would be to define conversion formulas for each temperature scale with just one canonical scale (say Kelvin).

The way that the "equations" are written places a burden on you of maintaining consistency. The values of temp_equations must appear in the same order as in temp_list, which must list the scales in the same order as they appear in the menu. If you ever want to change the menu order, good luck to you.

You could eliminate temp_list by using an OrderedDict. You can also generate the text menu from that dictionary, though the code to do so is a bit tricky.

Pay attention to spelling. You wrote "Reaumer" in temp_list, temp_equations, and the menu, despite having spelled it correctly in the comments. You also wrote "Celcius" and "What temperature are you converting too?" in the brute-force solution.

From a human-factors point of view, I think it's confusing to ask for the source scale, then the destination scale, and then the input temperature in source scale units. For coherence, it would be better to ask for the input temperature before asking for the destination scale.

To avoid all ambiguity, instead of asking "Enter the temperature degrees" or "Type the value for conversion", ask "Enter the temperature in Réaumur".

Suggested solution

```
from collections import OrderedDict

TEMP_SCALES = OrderedDict([
('Celsius', {
'from_K': lambda k: k - 273.15,
'to_K': lambda c: c + 273.15
}),
('Fahrenheit', {
'from_K': lambda k: 1.8 * k + 459.67,
'to_K': lambda f: (f + 459.67) / 1.8,
}),
('Kelvin', {
'from_K': lambda k: k,
'to_K': lambda k: k
}),
('Rankine', {
'from_K': lambda k: 1.8 * k,
'to_K': lambda r: r / 1.8
}),
('Delisle', {
'from_K': lambda k: 1.5 * (373.15 - k),
'to_K': lambda d: 373.15 - (2/3) * d
}),
('Newton', {
'from_K': lambda k: 0.33 * (k - 273.15),
'to_K': lambda n: (100 / 33) * n + 273.15
}),
('Réaumur', {
'from_K': lambda k: 0.8 * (k - 273.15),
'to_K': lambda r: 1.25 * r + 273.15
}),
('Rømer', {
'from_K': lambda k: (21 / 40) * (k - 273.15) + 7.5,
'to_

Code Snippets

# Selects the calculated equation for the selected conversion
temp_conversion = (select_equation[conversion_temp - 1])
def converter(function):
    # Assigned returned values variable names for readability.
    initial_temp = function[0]
    conversion_temp = function[1]
    temp = function[2]
def converter(from_scale, to_scale, from_temp):
    …
def main():
    converter(*declare_var())
temp_equations = {"Celsius": [temp,  # Celsius
                              temp * 9.0 / 5.0 + 32.0,  # Fahrenheit
                              temp + 273.15,  # Kelvin
                              temp + 273.15 * 9.0 / 5.0,  # Rankine
                              (100 - temp) * 3.0 / 2.0,  # Delisle
                              temp * 33.0 / 100.0,  # Newton
                              temp * 4.0 / 5.0,  # Réaumur
                              temp * 21.0 / 40.0 + 7.5],  # Rømer

Context

StackExchange Code Review Q#116748, answer score: 32

Revisions (0)

No revisions yet.