# What could I do differently to slim down my Python code?

Could I code differently to slim down the point of this Python source code? The point of the program is too get the users total amount and add it too the shipping cost. The shipping cost is determined by both country (Canada or USA) and price of product: The shipping of a product that is \$125.00 in Canada is \$12.00.

``````input ('Please press "Enter" to begin')

while True: print('This will calculate shipping cost and your grand total.')

totalAmount = int(float(input('Enter your total amount: ').replace(',', '').replace('\$', '')))

usa = "USA"
lessFifty = totalAmount <= 50
fiftyHundred = totalAmount >= 50.01 and totalAmount <= 100
hundredFifty = totalAmount >= 100.01 and totalAmount <= 150
twoHundred = totalAmount

if Country == "USA":
if lessFifty:
print('Your grand total is: \$',totalAmount + 6)
elif fiftyHundred:
print('Your grand total is: \$',totalAmount + 8)
elif hundredFifty:
print('Your grand total is: \$',totalAmount + 10)
elif twoHundred:

if lessFifty:
print('Your grand total is: \$',totalAmount + 8)
elif fiftyHundred:
print('Your grand total is: \$',totalAmount + 10)
elif hundredFifty:
print('Your grand total is: \$',totalAmount + 12)
elif twoHundred:

endProgram = input ('Do you want to restart the program?')
if endProgram in ('no', 'No', 'NO', 'false', 'False', 'FALSE'):
break
``````
-
possible duplicate of What could I add or subtract to my Python application? –  Quentin Pradet Jan 31 at 14:20

## migrated from programmers.stackexchange.comJan 31 at 13:16

This question came from our site for professional programmers interested in conceptual questions about software development.

I would not hard-code the fee logic, but instead store it as pure data. It's easier to maintain, even allowing to load it from a file. Then, it boils down to a range-based lookup, which is quite classical (cf `HLOOKUP` in spreadsheet software, with so called "approximate search").

In Python, we can perform such a search via `bisect`, relying on lexicographic order (and infinity as an unreachable upper bound).

Separated core logic would look like :

``````from bisect import bisect

#For each country, list of (x,y) = (cost_threshold, fee)
#For first x such cost <= x, pick y for fee.
inf = float("inf")
shippingFees = { 'USA' :    [ (50, 6), (100, 8),  (150, 10), (inf, 0) ],
'CANADA' : [ (50, 8), (100, 10), (150, 12), (inf, 0) ]
}
#Make sure it is sorted (required for lookup via bisect)
for fee in shippingFees.values() : fee.sort()

def getShippingFee(amount, country):
idx = bisect(fees, (amount,) )
return fees[idx][1]
``````

## Update

Here is a sample of "working application" using the helper function, assuming you have saved the code snippet above as `prices.py` (which should be stored in a module, but that's another story).

NB : I have dropped the exit part, since I don't like to type `no` when I can hit CTRL+C.

``````#!/usr/bin/python2

from prices import getShippingFee

print('This will calculate shipping cost and your grand total.')

while True:

#TODO : proper input processing, python3 compatible.
totalAmount = float(raw_input('Enter your total amount: ').replace(',', '').replace('\$', ''))

try :  #TODO : proper country check.
shippingFee = getShippingFee(totalAmount, country)
grandTotal = totalAmount + shippingFee
if shippingFee :
print('Your shipping cost is: %.2f' % shippingFee)
else :
print('Your grand total is: %.2f' % grandTotal)

except KeyError :
print ("Sorry, we don't ship to this hostile country : %s" % country)
``````
-
Could you explain more into your script? I don't understand how to make it work into a working application. –  JordanSimps Jan 31 at 12:36
This is a great answer especially in a production environment where the code is liable to be changed by someone other than the original author. Someone could easily guess how to add another country or modify the fees structure. –  sotapme Feb 1 at 10:42
@sotapme Thank you ! The main benefit is that the very same data structure can be reused, to generate shipping cost table in web page for instance. This follows DRY principle : Every piece of knowledge must have a single, unambiguous, authoritative representation within a system. –  John Optional Smith Feb 1 at 19:04
• Your example is broken. I don't think `while True: print(...)` is what you are looking for.
• I cannot see why you are eliminating all the commas from the input `totalAmount` and use `int(float(...))` anyway.
• Your `elif twoHundred:` will fail for `0`.

I suggest to put input validation into a for loop with some error handling. Just as a recommendation.

``````# Get total amount and country
while True:
try:
total_amount = input('Enter your total amount: ')
total_amount = total_amount.replace('\$', '').replace(',', '.')
total_amount = float(total_amount)
break
except ValueError:
print('Invalid floating point value')
``````

More importantly, it's considered best practice to compare lowercase strings to allow the user to not care about case sensitivity. You are currently not using the `usa` and `canada` variables.

``````usa = "usa"

if Country.lower() == usa:
...
...

if endProgram.lower() in ('no', 'false'):
break
``````

And Dennis' answer helps as well.

``````#!/usr/bin/env python3

print('This will calculate shipping cost and your grand total.')
input('Please press "Enter" to begin ')

# Get total amount and country
while True:
try:
total_amount = input('Enter your total amount: ')
total_amount = total_amount.replace('\$', '').replace(',', '.')
total_amount = float(total_amount)
break
except ValueError:
print('Invalid floating point value')

# evaluation of user data
usa = "usa"
less_fifty = total_amount <= 50
fifty_hundred = total_amount > 50.0 and total_amount <= 100
hundred_fifty = total_amount > 100.0 and total_amount <= 150

if country == usa:
if less_fifty:
shipping = 6
elif fifty_hundred:
shipping = 8
elif hundred_fifty:
shipping = 10
else:
shipping = None

if less_fifty:
shipping = 8
elif fifty_hundred:
shipping = 10
elif hundred_fifty:
shipping = 12
elif twoHundred:
shipping = None

# print output
if shipping is None:
shipping = 0
else:
print('Your grand total is: \$', total_amount + shipping)

endProgram = input ('Do you want to restart the program? ')
if endProgram.lower() in ('no', 'false'):
break
``````
-
Damn, a duplicate. I don't think it's on purpose, he was told to go here, and his previous question has been automatically migrated too. Voting to close this one, please answer to the other question which has been around for a longer time. –  Quentin Pradet Jan 31 at 14:20

Just to add that you can do `a <= x < y` in Python, which to me is a lot easier to write and read than setting up three variables just to say what range the total amount is within:

``````shipping = None
if total_amount <= 50:
shipping = 6
elif 50 < total_amount <= 100:
shipping = 8
elif 100 < total_amount <= 150:
shipping = 10
if country == 'canada' and shipping:
shipping += 2
``````
-
• First thing is you don't perform much test on the user input. That's fine if that's just a little tool for yourself but I you plan to make it available for other users, it might be a good idea to have a few more checks.

• You are storing "USA" and "Canada" in variables which looks like a good idea. However, you don't use it later on. You could do something like : `Country = str(input('Type "', canada, '" for Canada and "', usa, '" for USA: '))` and then `if Country == canada:` and `if Country == usa:`

• A string can't be both "Canada" and "Usa". Thus, `if Country == canada:` could become `elif Country == canada:`

• As this "print('Your shipping is: \$X.00') print('Your grand total is: \$',totalAmount + X)" is repeted many times. It would probably be a could thing to make it a function taking a shippingFee and totalAmount as arguments and doing all the printing for you. Otherwise, instead of having the printing everywhere, you could just update a shippingFee variable and make the printing at the end.

• I'm not sure that storing the result of the comparison in variables is a really good idea, especially if you give them names containing written numbers : if you want to change one thing, you'll end up changing everything. On top of that, your conditions are dependant on each other : if you've already checked that `totalAmount <= 50`, if your elif condition, you already know that `totalAmount > 50`. Thus, your logic could be : `if totalAmount <= 50: foo elif totalAmount <=100: bar elif totalAmount <= 150: foobar else: barfoo`

• Other variables have non conventional names. For instance, `country` would be better than `Country`

• It might be interesting to check the amount before considering the country.

In conclusion, the code could be like (it doesn't behave exactly like your and I haven't tested it):

``````usa = "USA"

input ('Please press "Enter" to begin')
while True:
print('This will calculate shipping cost and your grand total.')

totalAmount = int(float(input('Enter your total amount: ').replace(',', '').replace('\$', '')))
country = str(input('Type "', canada, '" for Canada and "', usa, '" for USA: '))
if country not in (canada, usa):
print "Invalid country"
continue

shippingFee = 0
if totalAmount<50:
shippingFee = 6
elif totalAmount < 100:
shippingFee = 8
elif totalAmount < 150:
shippingFee = 10

if shippingFee != 0 and country == canada:
shippingFee = shippingFee+2

if shippingFee: # this could be using a ternary operator but that's a detail
else:
print('Your grand total is: \$',totalAmount + shippingFee)

endProgram = input ('Do you want to restart the program?')
if endProgram in ('no', 'No', 'NO', 'false', 'False', 'FALSE'):
break
``````
-
`if shippingFee` should probably be `if not shippingFee`. For consistency, `if shippingFee != 0` can also be written as just `if shippingFee`. Rather than listing all case variations of 'no' and 'false', just use call `lower()` on `input`. On a final note, in Python generally underscore_style is preferred over camelCase. –  Thijs van Dien Jan 31 at 8:57
line 9, in <module> country = str(input('Type "', canada, '" for Canada and "', usa, '" for USA: ')) TypeError: input expected at most 1 arguments, got 5 –  JordanSimps Jan 31 at 9:58

First make sure the script work: proper indenting, shebang, correct logic. When the user enters 170\$, you give him free shipping only because `twoHundred` evaluates to `totalAmount` which evaluates to true, so it seems simple to just say "otherwise, it's free". You also don't handle errors. I decided to handle the "wrong country" error, but many things could go wrong when inputing the total amount. As other said, first separate printing from application logic:

``````#!/usr/bin/env python3
#-*- coding: utf-8 -*-

input ('Please press "Enter" to begin')

while True:
print('This will calculate shipping cost and your grand total.')
totalAmount = int(float(input('Enter your total amount: ') \
.replace(',', '').replace('\$', '')))

lessFifty = totalAmount <= 50
fiftyHundred = totalAmount >= 50.01 and totalAmount <= 100
hundredFifty = totalAmount >= 100.01 and totalAmount <= 150

if Country == "USA":
if lessFifty:      shippingCost = 6
elif fiftyHundred: shippingCost = 8
elif hundredFifty: shippingCost = 10
else:              shippingCost = 0
if lessFifty:      shippingCost = 8
elif fiftyHundred: shippingCost = 10
elif hundredFifty: shippingCost = 12
else:              shippingCost = 0
else:
print("Unknown country.")
break

print('Your grand total is: \${:.2f}'.format(totalAmount + shippingCost))

endProgram = input ('Do you want to restart the program?')
if endProgram in ('no', 'No', 'NO', 'false', 'False', 'FALSE'):
break
``````

You can notice that the if blocks are still repetitive. You can use a dictionary to store the shipping costs.

``````#!/usr/bin/env python3
#-*- coding: utf-8 -*-

input ('Please press "Enter" to begin')

while True:
print('This will calculate shipping cost and your grand total.')
totalAmount = int(float(input('Enter your total amount: ') \
.replace(',', '').replace('\$', '')))

if country not in ['Canada', 'USA']:
print("Unknown country.")
break

if totalAmount <= 50: amountCategory = 'lessFifty'
elif totalAmount >= 50.01 and totalAmount <= 100: amountCategory = 'fiftyHundred'
elif totalAmount >= 100.01 and totalAmount <= 150: amountCategory ='hundredFifty'
else: amountCategory = 'free'

shippingCosts = {
'USA': {'lessFifty': 6, 'fiftyHundred': 8, 'hundredFifty': 10, 'free': 0},
'Canada': {'lessFifty': 8, 'fiftyHundred': 10, 'hundredFifty': 12, 'free': 0}
}

shippingCost = shippingCosts[country][amountCategory]

print('Your grand total is: \${:.2f}'.format(totalAmount + shippingCost))

endProgram = input ('Do you want to restart the program?')
if endProgram in ['no', 'No', 'NO', 'false', 'False', 'FALSE']:
break
``````

I also used a list instead of a tuple to check for endProgram since it's more idiomatic. I don't think there's much more to do at this point to reduce size. You could find a smarter way to compute the shipping cost (see Kolmogorov complexity) but that would make it harder to change the rules for no good reason. The block which compute amountCategory isn't too nice but since `if`s are not expressions in Python you can't write `amountCategory = if ...: 'lessFifty' else: 'free'`. You could use a function if you wanted.

-

Sure you can condense some things, and improve the input loop. But I find your original code easy to read, whereas this is probably harder to read :

``````banner                  = 'Now we\'ll calculate your total charge, including shipping.'
cost_prompt             = 'Enter your total purchase amount : '
country_prompt          = 'Where are you shipping to ?\nEnter "USA" for the USA or "Canada" for Canada : '
shippingis              = 'Your shipping is '
grandtotalis            = 'Your grand total is: '
exit_prompt             = 'Type q to quit, or enter to continue: '
shiplaw                 = {
'USA' : {
0 : 6, 1 : 8, 2 : 10, 3 : 0
},
0 : 8, 1 : 10, 2 : 12, 3 : 0
}
}

def get_int(prompt):
try:        return int(float(raw_input(prompt).replace(',','').replace('\$','')))
except:     return None

def dollars_n_cents(*vals):
return '\$' + str(int(sum(vals))) + '.00'

def response(total,shipping):
shippingstr = 'free!' if shipping == 0 else dollars_n_cents(shipping)
return (shippingis + shippingstr, grandtotalis + dollars_n_cents(total,shipping))

def run():
while True:
print banner
total_amount = get_int(cost_prompt)
while total_amount is None: total_amount = get_int(cost_prompt)
country_str = raw_input(country_prompt)
while country_str.upper() not in shiplaw: country_str = raw_input(country_prompt)
result = response(total_amount,shiplaw[country_str.upper()][min(3,total_amount/50)])
print '\n'.join(result)
try :
if raw_input(exit_prompt)[0] == 'q': break
except : pass

import sys
if __file__ == sys.argv[0]:
run()
``````
-

One thing you could do is to calculate the shippingCost first, and then print with this logic

``````if shippingCost = 0
print: free shipping
else