CS50P coke pset.. I tried many ways to fix this but I can't seem to get right , how can i fix this?

Question

Price = 50

def main():
    coins = 0  
    if coins < price:
        print("Amount Due: ", due_amount(coins))  
    else:
        coins >= price  
        print("Change Owned: ", change(coins))

def due_amount(coins):
    
    while coins = (5, 10, 25) and coins < price:
        ipt = int(input("Insert coins: "))
        coins += ipt
        due = price - coins
 
    return coins

def change(coins):
    left = coins - price
    return left

main()

Answer 1

Easy Fixes

• price is defined up top as Price but then you use price these won't match this code won't run and you'd get an error if you tried running locally. You need to fix the top one to be price but don't do this... Avoid global variables, the point to learning how to use functions is to learn how to pass variables around. So move this into main and if you really want to use it in another function, pass the variable properly.

Main problem

Put politely, your program even if you corrected amount_due still won't work because of the if/else you use in main that shouldn't be there. You dont have a clear separation of what logic is for main and what logic is to be done elsewhere. Even worse, you overly rely on global variables for some parameters but not others.

I'd suggest you restructure your main keeping the following in mind:

1. Re-read the instructions.
2. Don't use globals not good for testing
3. What is the input / output cycle supposed to be?
4. Are there only certain values you should be accepting for user input? (Hint YES)
   1. What should you do if the value isn't allowed?
5. When does the program end?
6. Aim to have functions do one conceptual thing and one thing only, it could be doing some work or primarily calling other functions.

Then ask yourself:

• Which repetitive tasks should be put into functions?
  • I'd imagine prompting the user, taking in user input, and validating it would be a good candidate for a function. It's a repeatable body of code. Right?
  • maybe other things too, maybe not
• Which things belong in main or another function
  • calling that other function at a minimum... but when? (this leads to next item)
• Where should the looping occur?
  • we know this will involve looping, where is a clean place for the looping to live that makes it easy to follow the code?
• How can I make the functions small enough that they are easy to test and understand?
  • The smaller and simpler a function is the easier it is to test it and know that it does the right thing. Functions then become like lego blocks where you can put them together to build more complex things. Functions are at their best when the name tells you almost exactly what the function does / handles / models. That way anyone reading your code can figure out what it should do, and quickly verify it does so, understand how to use it and reuse it etc.

My conclusions... think about yourself before reading this

To me this suggests that I'd probably have a function fully handle the user input. Rather than name it get_input I'm going to call it what it represents the user doing. In this case insert_coin because it's basically the logic we use to prompt the user to insert a coin, and then validate the input, and return back to main the result.
I'd then leave the looping to be done out in main, so I don't actually need to pass any variables around. insert_coin doesn't have to know the total it just knows what's allowed from the user.

Ok so your main

• You have an if/else in main that shouldn't be there. coins starts as 0, so 0 will be less than price unless price is 0 or negative; you will always take the true branch and call print and asks for amount. You will NEVER call change and you will never take the other branch of the if statement.

A Thought on Design

I would generally suggest that you write your main function first, calling helper functions where appropriate and initially assume they will do what they need to do. And then go back and implement the helper functions to make them do the right thing. This way you defer logic from main that you see belongs elsewhere and might be repeated because your main resembles an outline AND you avoid implementing details too early, until you have determined how it will fit together.

You can then hardcode different functions to potentially return 'known values' to verify that main is good, and then one by one, go back and put the correct implementation into the function. Test the function and etc.

Back to main

Your main should probably look something like this.. don't peak until you try something on your own. There are other possibilities, but they will be somewhat similar, a key is do not use globals that defeats a large point of using functions to begin with.

 def main():
    # price of a soda
    price = 50
    # how much the user has paid the machine
    paid = 0

    # until the user has put enough money in to pay the price loop
    while CONDITION : # replace with condition; it involves price and paid
        print("Amount Due: ", SOMETHING) # SOMETHING is how much they still owe
        # We will increase the total amount they have paid by only proper values of coins
        paid += insert_coin()
    # user paid enough to purchase the soda, and might be owed change
    print("Change Owed: ", str(paid - price) # you already figure this part out
 

Hints for insert_coins or a helper function

• try running locally, add prints see what's doing what it should/shouldn't be happening

In your primary loop you keep resetting coins to be a list, but then you add to the list in the loop, only to reset it again... this will never work. coins can't be both a list and scalar (for comparison with price) at the same time.

(You also store a result into a variable due but you don't do anything with this variable. let's just get rid of that)

It looks like you were thinking that with this code

while coins = (5, 10, 25) and coins < price:

That you were making sure coins was one of those values AND coins was less than price to control the loop. However this has major problems. Not the least of which is that if it DID do that, your initial value 0, would not be accepted and would break the loop, and even if it didn't blow up on that, you only read the user input AFTER doing this check... which isn't what you want either.

You can test for a value being in a group of allowed values using a list and a test for list membership like so:

a = ...
b = ...
c = ...
my_items = (a, b, c)  # constructs a list of variables a, b and c 

if x in my_items:  # if x is an item in the list my_items
    do_something()

# in this case you get input from user... and it's a string and you want to return a number so perhaps....

allowed_coins = {"5", "10", "25"}
... # prompt user 
if x not in allowed_coins:
    return 0
else:
    return int(x)