premium_cost
There is no reason to include the return of this premium_cost in ground_shipping_cost. Use a different method here
DRY
You are correct. There are too many if statements. If you want to introduce an new threshold for the cost, this will include a adding an extra clause, and changing the limits for the preceding and following thresholds. This is just waiting for errors to pop up. Better would be to keep a dict of the thresholds, and iterate over them to get this price factor:
def ground_shipping_cost(weight):
thresholds = {2: 1.5, 6: 3.0, 10: 4.0, float("inf"): 4.75}
flat_cost = 20
for threshold, factor in sorted(thresholds.items()):
if weight <= threshold:
break
return flat_cost + weight * factor
thresholds is a dict, with the cost per weight unit as value, and the threshold as key.
The drone_shipping_cost can be tackled comparably.
Now you have 2 methods that, starting from a list of thresholds, tries to determine the cost factor. We can easily refactor this out:
def get_factor(thresholds, value):
for threshold, factor in sorted(thresholds.items()):
if value <= threshold:
return factor
def ground_shipping_cost(weight):
thresholds = {2: 1.5, 6: 3.0, 10: 4.0, float("inf"): 4.75}
flat_cost = 20
return flat_cost + get_factor(thresholds, weight)
def drone_shipping_cost(weight):
thresholds = {2: 4.5, 6: 9.0, 10: 12.0, float("inf"): 14.75}
return weight * get_factor(thresholds, weight)
Cheapest costs
Your cheapest_shipping method calculates the costs for the different shipping methods, finds the cheapest one and formats this into a string. This string formatting is also very repetitive, and should be done somewhere else. cheapest_shipping will be the most clear if it only returned which method is the cheapest, and the corresponding cost. This also allow you to test this method with unit tests further on.
Since methods are real objects in Python, and you can pass references to them on and store these in dicts, calculating the costs for the different methods can be simplified a lot:
def cheapest_shipping(weight):
methods = {
"drone": drone_shipping_cost,
"standard ground": ground_shipping_cost,
"premium ground": lambda weight: 125,
}
results = {method: calculation(weight) for method, calculation in methods.items()}
To look for the cheapest option among those, you can use the built-in min:
cheapest_method = min(results, key=lambda method: results[method])
return cheapest_method, results[cheapest_method]
Note: the lambda weight: 125 is the equivalent of
def premium_shipping(weight):
return 125
and "premium ground": premium_shipping, in the methods dict
And this can be called and formatted using str.format or f-strings in Python 3.6+
method, cost = cheapest_shipping(4)
f"You should use {method} shipping as it will only cost {cost}"
'You should use standard ground shipping as it will only cost 23.0'
Further refactoring
You can even refactor this one step further, and have 1 generalized cost calculation method that take a flat_cost and thresholds as arguments
def get_shipping_cost(weight, thresholds=None, flat_cost=0):
if thresholds is None:
return flat_cost
return flat_cost + weight * get_factor(thresholds, weight)
then you can define the different shipping methods like this:
shipping_methods = {
"drone": {"thresholds": {2: 4.5, 6: 9.0, 10: 12.0, float("inf"): 14.75}},
"standard ground": {
"flat_cost": 20,
"thresholds": {2: 1.5, 6: 3.0, 10: 4.0, float("inf"): 4.75},
},
"premium ground": {"flat_cost": 125},
}
def cheapest_shipping2(methods, weight):
results = {
method: get_shipping_cost(weight, **parameters)
for method, parameters in methods.items()
}
cheapest_method = min(results, key=lambda method: results[method])
return cheapest_method, results[cheapest_method]
method, cost = cheapest_shipping2(shipping_methods, 4)
different min
Instead of using min with the cost as key, you can reverse the results dictionary:
def cheapest_shipping2(methods, weight):
results = {
get_shipping_cost(weight, **parameters): method
for method, parameters in methods.items()
}
cost, method = min(results.items())
return method, cost
In case of an ex aequo, this can have different results than the other method
even further refactoring.
Now all shipping methods are calculated with get_shipping_cost. If you want to use different functions for the different shipping methods, you can do something like this:
def cheapest_shipping3(methods, weight, default_cost_method=get_shipping_cost):
results = {
parameters.pop("method", default_cost_method)(
weight, **parameters
): method
for method, parameters in methods.items()
}
cost, method = min(results.items())
return method, cost
shipping_methods2 = {
"drone": {"thresholds": {2: 4.5, 6: 9.0, 10: 12.0, float("inf"): 14.75}},
"standard ground": {
"flat_cost": 20,
"thresholds": {2: 1.5, 6: 3.0, 10: 4.0, float("inf"): 4.75},
},
"premium ground": {"method": lambda weight: 125},
}
method, cost = cheapest_shipping3(shipping_methods2, 4)
please note that the parameters.pop mutates the original shipping_methods2 after the execution:
{
"drone": {"thresholds": {2: 4.5, 6: 9.0, 10: 12.0, inf: 14.75}},
"standard ground": {
"flat_cost": 20,
"thresholds": {2: 1.5, 6: 3.0, 10: 4.0, inf: 4.75},
},
"premium ground": {},
}
To prevent this, we need to make a copy of the methods:
def cheapest_shipping3(methods, weight, default_cost_method=get_shipping_cost):
methods_copy = {
method: parameters.copy()
for method, parameters
in methods.items()
}
results = {
parameters.pop("method", default_cost_method)(
weight, **parameters
): method
for method, parameters in methods_copy.items()
}
cost, method = min(results.items())
return method, cost
