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 - 2\n")
choice=int(input("Choose a conversion by typing a number: "))
#If 1 is chosen follows by asking what metric distance to convert
if choice== 1:
print("Please choose the measurement you want to use.\n")
#prints the metric length menu
print(metric_length)
distance =input("Type an option from the menu: ")
#if statements that takes user input to assign a value for leng1
#then runs the function
if distance=="km":
leng1=int(input("Please enter a number to convert: "))
print=(km_mi())
elif distance=="m":
leng1=int(input("Please enter a number to convert: "))
print=(m_ft())
elif distance=="cm":
leng1=int(input("Please enter a number to convert: "))
print=(cm_in())
else:
print("Invalid Entry")
#This is the code for the second choice in the program
#If 2 is chosen follows by asking what imperial distance to convert
if choice== 2:
print("Please choose the measurement you want to use.\n")
#prints the imperial length menu
print(imperial_length)
distance2 =input("Type an option from the menu: ")
#if statements that takes user input to assign a value for leng2
#then runs the function
if distance2=="mi":
leng2=int(input("Please enter a number to convert: "))
print=(mi_km())
elif distance2=="ft":
leng2=int(input("Please enter a number to convert: "))
print=(ft_m())
elif distance2=="in":
leng2=int(input("Please enter a number to convert: "))
print=(in_cm())
else:
print("Invalid Entry")
if measure==2:
#menu to chose from either metric to imperial or vice-versa
print("Metric-Imperial - 1\n")
print("Imperial-Metric - 2\n")
choice2=int(input("Choose a conversion by typing a number: "))
#If 1 is chosen follows by asking what metric distance to convert
if choice2== 1:
print("Please choose the measurement you want to use.\n")
#prints the metric length menu
print(metric_mass)
weight =input("Type an option from the menu: ")
#if statements that takes user input to assign a value for mass1
#then runs the function
if weight=="t":
mass1=int(input("Please enter a number to convert: "))
print=(t_st())
elif weight=="kg":
mass1=int(input("Please enter a number to convert: "))
print=(kg_lb())
elif weight=="g":
mass1=int(input("Please enter a number to convert: "))
print=(g_oz())
else:
print("Invalid Entry")
#This is the code for the second choice in the program
#If 2 is chosen follows by asking what imperial mass to convert
if choice2== 2:
print("Please choose the measurement you want to use.\n")
#prints the imperial length menu
print(imperial_mass)
weight2 =input("Type an option from the menu: ")
#if statements that takes user input to assign a value for mass2
#then runs the function
if weight2=="stone":
mass2=int(input("Please enter a number to convert: "))
print=(st_t())
elif weight2=="lb":
mass2=int(input("Please enter a number to convert: "))
print=(lb_kg())
elif weight2=="oz":
mass2=int(input("Please enter a number to convert: "))
print=(oz_g())
else:
print("Invalid Entry")
4 Answers 4
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
<system>_<quantity>
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 alengX
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 bothkm_mi()
andmi_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 becauseprint
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
?
One idea: Combine all your similar functions into one function and use parameters in your calls ...
def convert_multiply(mass, factor, label):
print (mass*factor, label)
Then, you can have a dictionary with the factors and labels ...
factor_labels = {'t': [157.47, "stone"],
'kg': [2.2, "pounds"],
...
}
A few things you can do:
Whenever you have to execute a few things after a loop (a conditional branch execution), this is the prime candidate to use a dictionary. In Python, functions are just names of code blocks; so you can use them anywhere you can use any other variable.
You only need to ask for the unit and the amount once after the correct selection has been made.
Here is a version of your code that does this; I have left your comments in place:
metric_length=['km', 'm' , 'cm']
imperial_length=['mi', 'ft', 'in']
menu_type=['Length-1', 'Weight-2', 'Cacpacity-3']
metric_mass=['t','kg', 'g']
imperial_mass=['stone', 'lb', 'oz']
converters = dict()
converters['km'] = lambda x: '{} {}'.format(x/1.6, 'miles')
converters['m'] = lambda x: '{} {}'.format(x*3.26, 'feet')
converters['cm'] = lambda x: '{} {}'.format(x/2.54, 'inches')
converters['mi'] = lambda x: '{} {}'.format(x*1.6, 'kilometers')
converters['ft'] = lambda x: '{} {}'.format(x/3.26, 'meters')
converters['in'] = lambda x: '{} {}'.format(x*2.54, 'centimeters')
converters['t'] = lambda x: '{} {}'.format(x*157.47, 'stone')
converters['kg'] = lambda x: '{} {}'.format(x*2.2, 'pounds')
converters['g'] = lambda x: '{} {}'.format(x*0.035, 'ounces')
converters['stone'] = lambda x: '{} {}'.format(x*0.00635, 'tonnes')
converters['lb'] = lambda x: '{} {}'.format(x*0.4536, 'kilograms')
converters['oz'] = lambda x: '{} {}'.format(x*28.35, 'grams')
print("Welcome to the Conversion-Calculator")
print('\n'.join(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")
print("Imperial-Metric - 2")
choice=int(input("Choose a conversion by typing a number: "))
#If 1 is chosen follows by asking what metric distance to convert
if choice== 1:
print('\n'.join(metric_length))
#This is the code for the second choice in the program
#If 2 is chosen follows by asking what imperial distance to convert
if choice== 2:
print('\n'.join(imperial_length))
#prints the imperial length menu
if measure==2:
#menu to chose from either metric to imperial or vice-versa
print("Metric-Imperial - 1\n")
print("Imperial-Metric - 2\n")
choice2=int(input("Choose a conversion by typing a number: "))
#If 1 is chosen follows by asking what metric distance to convert
if choice2== 1:
print('\n'.join(metric_mass))
#prints the metric length menu
#This is the code for the second choice in the program
#If 2 is chosen follows by asking what imperial mass to convert
if choice2== 2:
print('\n'.join(imperial_mass))
option = input("Please choose the measurement you want to use: ")
if option not in converters.keys():
print('Invalid Entry')
else:
leng1=int(input("Please enter a number to convert: "))
print(converters[option](leng1))
I also made the method names the same as the inputs, this way if the user enters something invalid, the key in the converters dictionary will not exist - an "cheap" way to get the validation done.
The main line that does the execution is here:
print(converters[option](leng1))
It is fetching the value from converters, which is a lamdba (a function you can write as an expression), and then calls it with the value entered by the user.
Here is a simplified example, because this is the main part that did the "shrinking" of your code.
Lets say you have two methods, a and b, and depending on what the user selects, you have to call either a or b. You might write some code like this:
def a():
return 'Hello'
def b():
return 'Goodbye'
user_request = input('Please enter either a or b: ')
if user_request == 'a':
print(a())
if user_request == 'b':
print(b())
This is fine if its two options, but what if there are 3, 4 or 9 options? Now you'll end up with a large chain of if/else statements. To prevent this, what you can do is use a call map - which is a dictionary that has the values as the method names:
call_map = dict()
call_map['a'] = a
call_map['b'] = b
As methods are just like normal values, you can assign and pass them around. Now what you want to do is call the method that corresponds to the input:
option = 'a'
function_to_call = call_map[option]
result = function_to_call()
You can shorten the above with:
result = call_map[option]()
As the dictionary only has keys for those values that map to valid functions, it is easy to filter out an invalid entry - it simply won't be in the list of keys for that dictionary.
if option not in call_map.keys():
print('{} is not a valid option'.format(option))
else:
print('The result is: {}'.format(call_map[option]())
-
\$\begingroup\$ Thanks for this it was really helpful, but I dont uderstand how you used the curly braces and the ".format" function with the lamda function and added a string to it. How does this work and what does the .format do? \$\endgroup\$Random Guy– Random Guy2014年10月23日 18:22:36 +00:00Commented Oct 23, 2014 at 18:22
-
\$\begingroup\$ Have a look at string formatting in the Python manual. \$\endgroup\$Burhan Khalid– Burhan Khalid2014年10月25日 14:39:53 +00:00Commented Oct 25, 2014 at 14:39
I see two obvious things you can do but I don't have the time to look into it in detail:
1: You try to assign a value to print. That causes an error.
2: You repeat equal lines of code in each condition of your if statements. Don't do that. Use one line that precedes all these if-statements. For example:
mass1=int(input("Please enter a number to convert: "))
if weight=="t":
print=(t_st())
elif weight=="kg":
print=(kg_lb())
elif weight=="g":
print=(g_oz())
else:
print("Invalid Entry")
If you don't want to read input in case the entry was wrong, you can use:
if weight not in {'t','kg','g'}:
print("Invalid Entry")
return
else:
mass1=int(input("Please enter a number to convert: "))
if weight=="t":
print=(t_st())
elif weight=="kg":
print=(kg_lb())
elif weight=="g":
print=(g_oz())
('g', 'oz')
would map to the functiong_oz
. Then, instead of all thatif
/elif
stuff, you just dofunctions[source, target]()
. \$\endgroup\$leng1
and the like, pass them as parameters to the functions. This won't make things that much shorter, but it will make them a whole lot simpler to understand, and to refactor. \$\endgroup\$functions[source, target](value)
it's justprint(conversions[source, target] * value, target)
. \$\endgroup\$