Shipping charge based on weight

Question

The program here is simply supposed to tell you the shipping charge based off of the weight of the package. I am just wondering if there is any further way to concise it? At the moment, I do not think it can go any further than that. The only place where I'm unsure whether I could improve it, could be Booleans. Possibly use a while statement?

def main():

    weight = eval(input("Please enter the weight of the package: "))

    rate = 0

    if weight <= 2:
        rate += 1.25
    elif weight > 2 and weight <= 5:
        rate += 2.35
    elif weight > 5 and weight <= 10:
        rate += 4.05
    elif weight > 10:
        rate += 5.55

    charge = rate * weight

    print("The shipping charge is: $%s" % charge)

print(main())

Answer 1

Here are some more thoughts, although some of it has been mentioned before:

• Calculating shipping charge sounds like a function – This sounds like something you could do multiple times, and as such should be made into a function
• Trust in the if / elif ... – It is unneccessary to do both the if weight <= 2 and the elif weight > 2.
• ... or alternatively use a dictionary/list and a loop – When you build repeating structures as the multiple if & elif you should consider if you can make this into a loop structure, which in general looks nicer than the repeated structure.
• Seperate business logic from presentation logic – In your case remove the print() arount the calling to main, and simplify main() to handle input/ouput, and let the function do the logic
• Add the tagging to allow use as a module – If you add the if __name__ == '__main__': block/tagging, you could reuse your module from another place when you want to calculate shipping charges
• Use int(input(..)) for integer input, float(input(...)) or just input(...) for string – These allows for better handling of your input. Also note that if you are using Python 2, you should use raw_input() instead of input().
• Change to the newer string.format – In Python 3 it is preferred to stop using the % operator, and rather use the string formatting. I.e. doing stuff like: "The shipping charge is: ${:.2f}".format(charge) which format the shipping charge as a float with 2 decimals.
• Document your code using docstrings/comments – Always nice to document constants or functions in your code!
• Include error handling – No point in calculating shipping charges of negative weiths for example. So a little input validation is always good

Code refactoring

# WEIGHT_RATES is a list of tuples = (min_weight, rate),
# As it is reverse sorted, look from start to find the first
# entry where your weight is higher than min_weight, and use
# corresponding rate
WEIGHT_RATES = [
      ( 10, 5.55),
      (  5, 4.05),
      (  2, 2.35),
      (  0, 1.25)
    ]

def shipping_charge(weight):
    """Return shipping charge based upon the weight."""

    if weight < 0:
       raise ValueError("Can't calculate shipping charge of negative weights")

    for min_weight, rate in WEIGHT_RATES:
        if weight > min_weight:
            return weight * rate


def main():
    """Read a single weight, and calculate shipping charge"""

    weight = int(input("Please enter the weight of the package: "))
    print('The shipping charge is: ${:,.2f}'.format(shipping_charge(weight)))


if __name__ == '__main__':
    main()

Added: shipping_charge_no_list

As commented upon, your teacher is not fond of lists, so here is a version of the function without the list. Note that, in my opinion, it is a little better to change the order around, mainly because I'll expect the first case to be most prominent and should break out of the if-block firstly. But that is a matter of taste, and your mileage may vary.

def shipping_charge_no_list(weight):
    """Return shipping charge based upon the weight."""

    if weight < 0:
       raise ValueError("Can't calculate shipping charge of negative weights")

    if weight > 10:
        rate = 5.55
    elif weight > 5:
        rate = 4.05
    elif weight > 2:
        rate = 2.35:
    else
        rate = 1.25

    return weight * rate

Answer 2

You are checking much more than you need to. Consider the first two cases:

if weight <= 2:
    # A
elif weight > 2 and weight <= 5:
    # B

If weight <= 2, we fall into case A. If we're not in case A, only then do we continue onto the next condition checks (because we're using elif and not if). As such, if we're even in the path that checks the condition which leads to B, we already know that weight > 2. Thus, that check is redundant and can be reduced to simply weight <= 5.

The whole body can become:

if weight <= 2:
    rate += 1.25
elif weight <= 5:
    rate += 2.35
elif weight <= 10:
    rate += 4.05
else:
    rate += 5.55

Which isn't just less code, it's easier to reason about since all your bounds are visually in the same column.

Answer 3

As others have commented on the logic, I'll comment on the rest :)

weight = eval(input("Please enter the weight of the package: "))

Why have you used eval()? It is used for running raw code from the input which you don't really need.

Instead, do:

int(input("Please enter the weight of the package: "))

Here, you convert the input to an integer, at the same time. If you want to use floats, simply do:

float(input("Please enter the weight of the package: "))

---

As far as I can see, there's no need to add to the rate because you're only calling the function once. So rate will always be 0 at the beginning.

So instead, remove the rate = 0 and change the rate += 1.25, etc... to rate = 1.25, etc...

---

There's also no need to print() main() because you have a print statement already in it.

Instead, you could return the string that you are currently printing, or just remove the surrounding print() from print(main())

---

With those changes in mind:

def main():

    weight = int(input("Please enter the weight of the package: "))

    if weight <= 2:
        rate = 1.25
    elif weight > 2 and weight <= 5:
        rate = 2.35
    elif weight > 5 and weight <= 10:
        rate = 4.05
    elif weight > 10:
        rate = 5.55

    charge = rate * weight

   return "The shipping charge is: $%s" % charge

print(main())

Answer 4

Since you have covered all possible cases, the last

elif weight > 10:
    rate += 5.55

can just be:

else:
    rate += 5.55

Also, in python, you can do:

elif 2 < weight <= 5:

Finally, your main doesn't return anything, so instead of

print(main())

you can just do:

main()

Answer 5

By extracting the computation of the rate to a different function, you can simplify the code by returning early when you know the rate:

def compute_rate(weight):
    if weight <= 2:
        return 1.25
    if weight <= 5:
        return 2.35
    if weight <= 10:
        return 4.05
    return 5.55

Used like this in main:

charge = compute_rate(weight) * weight

Answer 6

I like to separate logic from data, to make both more readable. Here, the key rate change points are in a separate dictionary. We can find the correct rate by checking from the end of the list. Shipping is calculated in the function, or we could return just the rate.

RATES = {
    0: 1.25,
    2: 2.35,
    5: 4.05,
    10: 5.55,
}

def calc_shipping(weight):
    # Make reversed key weight list
    key_weights = sorted(RATES.keys(), reverse=True)

    for more_than in key_weights:
        if weight > more_than:
            return RATES[more_than] * weight
    else:
        return 0.0

To test for the correctness of the solution compared to the original, I used this py.test parameterized function:

@pytest.mark.parametrize("weight, expected_shipping", [
    (0, 0),
    (1.5, 1.875),
    (2, 2.5),
    (2.0001, 4.700235),
    (4, 9.4),
    (5, 11.75),
    (5.0001, 20.250405),
    (8, 32.4),
    (10, 40.5),
    (15, 83.25),
])
def test_calc_shipping(weight, expected_shipping):
    # Check for approximate equality
    assert abs(calc_shipping(weight) - expected_shipping) < 0.0000001