8
\$\begingroup\$

I have made a program to calculate the amount of flour, eggs, sugar and butter required for making lemon cakes and cupcakes.

import math

cupcake = [4, 0.1, 12, 14]
#[Butter, egg, flour, sugar]
lemon = [80, 4.5, 240, 300]
#[Butter, egg, flour, sugar]

cup_in = int(input('How many cupcakes would you like? '))
lem_in = int(input('How much lemon cake would you like? '))

amount_cup = []
amount_cup = [x*cup_in for x in cupcake]

amount_lemon = []
amount_lemon = [(x * lem_in) for x in lemon]

print(amount_cup)
print(amount_lemon)

total = []
total = [x + y for x, y in zip(amount_cup, amount_lemon)]

print(total)

amt_butter = total[0]
amt_egg = total[1]
amt_flour = total[2]
amt_sugar = total[3]


bag_but_large = 0
bag_but_medium = 0
bag_but_small = 0

bag_flr_large = 0
bag_flr_medium = 0
bag_flr_small = 0

bag_egg_large = 0
bag_egg_medium = 0
bag_egg_small = 0

bag_sgr_large = 0
bag_sgr_medium = 0
bag_sgr_small = 0

#Calculates the amount of butter bags needed
while amt_butter > 0:

    if amt_butter > 500:
        amt_butter -= 500
        bag_but_large += 1

    elif amt_butter > 250:
        amt_butter -= 250
        bag_but_medium += 1

    elif amt_butter > 125:
        amt_butter -= 125
        bag_but_small += 1        

    else:
        bag_but_small += 1
        amt_butter = 0

#Flour
while amt_flour > 0:

    if amt_flour > 750:
        amt_flour -= 750
        bag_flr_large += 1        

    elif amt_flour > 500:
        amt_flour -= 500
        bag_flr_medium += 1

    elif amt_flour > 250:
        amt_flour -= 250
        bag_flr_small += 1        

    else:
        bag_flr_small += 1
        amt_flour = 0

#Egg
    while amt_egg > 0:

        if amt_egg > 12:
            amt_egg -= 12
            bag_egg_large += 1            

        elif amt_egg > 10:
            amt_egg -= 10
            bag_egg_medium += 1

        elif amt_egg > 6:
            amt_egg -= 6
            bag_egg_small += 1

        else:
            bag_egg_small += 1
            amt_egg = 0

#Sugar
while amt_sugar > 0:

        if amt_sugar > 600:
            amt_sugar -= 600
            bag_sgr_large += 1

        elif amt_sugar > 400:
            amt_sugar -= 400
            bag_sgr_medium += 1

        elif amt_sugar > 200:
            amt_sugar -= 200
            bag_sgr_small += 1

        else:
            bag_sgr_small += 1
            amt_sugar = 0

print("*" * 80)
print("Purchases needed: ")
print(bag_but_large, "large bags of butter needed.")
print(bag_but_medium, "medium bags of butter needed.")
print(bag_but_small, "small bags of butter needed.")
print("*" * 80)
print(bag_flr_large, "large bags of flour needed.")
print(bag_flr_medium, "medium bags of flour needed.")
print(bag_flr_small, "small bags of flour needed.")
print("*" * 80)
print(bag_sgr_large, "large bags of sugar needed.")
print(bag_sgr_medium, "medium bags of sugar needed.")
print(bag_sgr_small, "small bags of sugar needed.")
print("*" * 80)
print(bag_egg_large, "large boxes of eggs needed.")
print(bag_egg_medium, "medium boxes of eggs needed.")
print(bag_egg_small, "small boxes of eggs needed.")
\$\endgroup\$
17
\$\begingroup\$

You should use functions and better datatypes.

You need to know how many; butter, egg, flour and sugar, bags you need, which come in large, medium and small sizes. And so wrote the same while loop four times. Which is perfect for a function!

Instead you should make a function if we keep it simple and have it work the way you have it now, then you can use a function like trickle_groups(amount: int, large: int, medium: int, small: int) -> tuple<int, int, int>. Which would get you the function:

def trickle_groups(amount, large, medium, small):
    large_bags, medium_bags, small_bags = 0, 0, 0
    while amount > 0:
        if amount > large:
            amount -= large
            large_bags += 1
        elif amount > medium:
            amount -= medium
            medium_bags += 1
        elif amount > small:
            amount -= small
            small_bags += 1
        else:
            small_bags += 1
            amount = 0
    return (large_bags, medium_bags, small_bags)

We can then use it on all the different ingredients:

butter_bags = trickle_groups(total[0], [500, 250, 125])
egg_bags    = trickle_groups(total[1], [12, 10, 6])
flour_bags  = trickle_groups(total[2], [750, 500, 250])
sugar_bags  = trickle_groups(total[3], [600, 400, 200])

Bam almost all your code is gone! After this you should want to do the same with your print statements. Make a format for the three duplicate print lines, and put it in a function.

DISPLAY_FORMAT = '''\
{1} large bags of {0} needed.
{2} medium bags of {0} needed.
{3} small bags of {0} needed.'''

def display_bags(type, bags):
    print(DISPLAY_FORMAT.format(type, *bags))

# ...

print("Purchases needed: ")
display_bags('butter', butter_bags)
display_bags('flour', flour_bags)
display_bags('sugar', sugar_bags)
display_bags('eggs', egg_bags)

After this I'd recommend that you rearrange the way that you go from input to output. Instead of manually looping, you can instead actually use a for loop. To do this you need to make a lists to loop through, so ingredients and bag sizes. After this you'd want to actually loop through them, with zip you can quite simply. And then you want to put it in a function main, which you call if you're in the main function:

INGREDIENTS = ['butter', 'egg', 'flour', 'sugar']
CUPCAKE = [4, 0.1, 12, 14]
LEMON = [80, 4.5, 240, 300]
BAG_SIZES = [
    [500, 250, 125],
    [12, 10, 6],
    [750, 500, 250],
    [600, 400, 200],
]

def main():
    cup_in = int(input('How many cupcakes would you like? '))
    lem_in = int(input('How much lemon cake would you like? '))

    amount_cup = [x*cup_in for x in CUPCAKE]
    amount_lemon = [x*lem_in for x in LEMON]
    total = [x + y for x, y in zip(amount_cup, amount_lemon)]
    bags = [trickle_groups(a, b) for a, b in zip(total, BAG_SIZES)]
    print("Purchases needed: ")
    for ingredient, bag in zip(INGREDIENTS, bags):
        display_bags(ingredient, bags)

if __name__ == '__main__':
    main()

You can also optimize trickle_groups, rather than looping until the amount is zero, you can instead find the amount of large bags you need, and the remainder with divmod. Where you get the amount you need with the division, and you get the amount left with the modulo part. I change it also to take any amount of bags, and so you should be able to get:

def trickle_groups(amount, bag_sizes):
    bag_sizes = list(bag_sizes)
    bag_sizes.sort(reverse=True)
    amount_bags = []
    for bag in bag_sizes:
        n, amount = divmod(amount, bag)
        amount_bags.append(n)
    if amount:
        amount_bags[-1] += 1
    return tuple(amount_bags)
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I'm curious about the function name trickle_groups. Did you come up with it? \$\endgroup\$ – Matthew Leingang Dec 9 '16 at 17:04
  • \$\begingroup\$ @MatthewLeingang Yes I did, if you have a better idea then I'm all ears! \$\endgroup\$ – Peilonrayz Dec 9 '16 at 17:08
  • \$\begingroup\$ Not yet. It reminds me of algorithms used to represent numbers in roman numerals. But I don't know of a good name for that, either. \$\endgroup\$ – Matthew Leingang Dec 9 '16 at 17:14
  • \$\begingroup\$ @MatthewLeingang I choose mine as it was kinda how I learnt, probably wrongly, trickle-down economics. The; upper, middle and lower, class have a predefined sized glasses, but the upperclass are at the top of this glass pyramid, and the 'water' trickles down. But I'm sure there's a better name, as it's not doing the same thing, :) \$\endgroup\$ – Peilonrayz Dec 9 '16 at 17:27
10
\$\begingroup\$
cupcake = [4, 0.1, 12, 14]
#[Butter, egg, flour, sugar]

The fact that you had to put a comment explaining what the four items are suggests its not clear enough on its own. Instead, build something to hold the four values; I'd suggest a namedtuple, as you don't need the facilities of a full class:

from collections import namedtuple

Ingredients = namedtuple('Ingredients', 'butter,egg,flour,sugar')

cupcake = Ingredients(butter=4, egg=0.1, flour=12, sugar=14)

Now you can access e.g. cupcake.egg too, and you get a nice printable representation.


cup_in = int(input('How many cupcakes would you like? '))

I'd suggest you put some validation around this input; see e.g. Asking the user for input until they give a valid response.


amount_lemon = []
amount_lemon = [(x * lem_in) for x in lemon]

The list comprehension creates a new list, so the first line is redundant.


After that, you have a lot of duplication. There is nothing different in the logic between assigning bags of e.g. flour and butter, just a difference in the values. I would refactor this into a function that takes those values, e.g.:

bag_egg_small, bag_egg_medium, bag_egg_large = subdivide(amt_egg, [6, 10, 12])

Similarly there's a lot of duplication in the printing out; try to reduce duplication, as it means that if you fix a bug or improve the functionality, everywhere that uses it benefits.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.