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

Conversions calculator

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

Problem

I'm a student currently studying computing/programming at school and am a beginner and would like some help understanding how to shorten down my code. I know some basic Python and can program relatively simple programs such as the one I need help with.My task/homework was to create a metric-imperial and vice-versa conversion calculator. I haven't quite finished it yet but how would I shorten down this long and repetitive code?

#Start
 print("Welcome to the Conversion-Calculator")


In this program, "_" means "to" eg. "km_mi" is kilometres to miles

```
#created the menus for the length
metric_length=["'km', 'm' , 'cm'"]
imperial_length=["'mi', 'ft', 'in'"]

#This defines variables from metric to imperial measurements of distance
def km_mi():
print(leng1/1.6, "miles")
def m_ft():
print(leng1*3.26, "feet")
def cm_in():
print(leng1/2.54, "inches")

#This list defines variables from imperial to metric measurements of distance
def mi_km():
print(leng2*1.6, "kilometers")
def ft_m():
print(leng2/3.26, "metres")
def in_cm():
print(leng2*2.54, "centimetres")

#menu for weights
metric_mass=["'t','kg', 'g'"]
imperial_mass=["'stone', 'lb', 'oz'"]

#this defines variables from metric to imperial in mass
def t_st():
print(mass1*157.47, "stone")
def kg_lb():
print(mass1*2.2, "pounds")
def g_oz():
print(mass1*0.035, "ounces")
#this defines variables from imperial to metric in mass
def st_t():
print(mass2*0.00635, "tones")
def lb_kg():
print(mass2*0.4536, "kilograms")
def oz_g():
print(mass2*28.35, "grams")

#creates startup menu
menu_type=["'Length-1', 'Weight-2', 'Cacpacity-3'"]

#choose which what to convert
print(menu_type)
measure=int(input("Please select what you would like to convert by typing a number:"))

#starting if statement chain
if measure==1:
#menu to chose from either metric to imperial or vice-versa
print("Metric-Imperial - 1\n")
print("Imperial-Metric -

Solution

There's too much code for me to do a line-by-line review, so here are a few general comments:

-
Read PEP 8, the Python style guide. Spaces around your binary operators will make reading your code easier.

-
Your _ variables seem odd to me. For example:

metric_length=["'km', 'm' , 'cm'"]


This is creating a list with a single element, and that element is the string 'km', 'm' , 'cm'. I think what you actually mean is to have a list of three elements like this:

metric_length_units = ['km', 'm', 'cm']


(Note also the change in variable name: this list contains unit names, not lengths.)

-
The functions km_mi(), m_ft(), and so on, don’t take any arguments, but the body includes a lengX variable. What if that isn’t set in the global scope? Instead, this should be passed in as an argument to the function.

-
Cacpacity is misspelt.

-
Every conversion factor appears at least twice. For example, 1.6 occurs in both km_mi() and mi_km(). One way to pull this out is to define a general purpose conversion function:

def convert_a_to_b(value, unit_a, unit_b):
    return value * conversion_rate(unit_a, unit_b)


where conversion_rate looks up the conversion rate (e.g. from a dict) that’s stored in one place. You could then define wrappers around this if you want, or just use it as-is.

Then take the value from this function, and drop that into a print string. Don’t print it directly: it makes it harder to pass values between different converters, and harder to test at a later stage.

-
I think I know what you meant to do here, but it’s wrong:

print=(cm_in())


I assume that stray equals sign snuck in by mistake. If not, you’re trying to assign to the variable print, which will fail because print is a reserved keyword.

-
You have the same code to ask the user to choose Imp->Met or Met->Imp at least twice. As you add more units, this will get duplicated again. Consider wrapping it in a function.

-
What happens if measure == 3?

Code Snippets

metric_length=["'km', 'm' , 'cm'"]
metric_length_units = ['km', 'm', 'cm']
def convert_a_to_b(value, unit_a, unit_b):
    return value * conversion_rate(unit_a, unit_b)
print=(cm_in())

Context

StackExchange Code Review Q#66923, answer score: 5

Revisions (0)

No revisions yet.