8
\$\begingroup\$

I'm new to programming and this is one of my first programs. I had no idea what I was doing so most of this program was made from bad bug-fixing that barely managed to fix the bugs. I'm sure that there are many things that I did wrong or very inefficiently. Please tell me what I did wrong and how I can improve this program:

stock={'banana':6,
'apple':0,
'orange':32,
'pear':15,}
prices={'banana': 4,
'apple':2,
'orange':1.5,
'pear':3}

def uppercase(x):
  return x[0].upper()+x[1:]

name=input('''What is your name?
''')
print('Hi, %s, welcome to my fruit store. Here is the menu:'%(name))
print()

def menu():
  for fruit in prices:
    print(uppercase(fruit))
    print('Price: $%s'%(prices[fruit]))
    print('Stock: %s'%(stock[fruit]))
    print()
  print('You have: $%s'%(money))
  print()

def ask_fruit(money):
  fruit=input('''What fruit do you want?
''')
  print()
  if fruit in stock:
    if stock[fruit]>0:
      ask_amount(fruit,money)
    else:
      print('''Sorry, %ss are out of stock
'''%(fruit))
      ask_fruit(money)
  else:
    print('''Sorry, we don\'t have that, look at the menu.
    ''')
    ask_fruit(money)

def ask_amount(fruit,money):
  amount=int(input('''How many %ss do you want?
'''%(fruit)))
  print()
  if amount<=0:
    print('''At least buy one.
  ''')
    ask_amount(fruit,money)
  elif stock[fruit]>=amount:
    sell(fruit,amount,money)
  else:
    print('''Sorry, we don\'t have that many %ss.
    '''%(fruit))
    ask_amount(fruit,money)

def sell(fruit,amount,money):
  cost=prices[fruit]*amount
  confirmation=input('''Are you sure? That will be $%s.
-Yes
-No
'''%(cost)).lower()
  print()
  if confirmation=='yes':
    money-=cost
    print('''Thank you for the business!
''')
    stock[fruit]=stock[fruit]-amount
    ask_again()
  elif confirmation=='no':
    ask_fruit()
  else:
    print('''Answer me.
''')
    sell(fruit,amount,money)

def ask_again():
  answer=input('''Do you want anything else?
-Yes
-No
''').lower()
  print()
  if answer=='yes':
    menu()
    ask_fruit(money)
  elif answer=='no':
    print('Okay, bye.')
  else:
    print('Answer me.')
    ask_again()

money=117
menu()
ask_fruit(money)
\$\endgroup\$
8
\$\begingroup\$

First of all well done, this is not bad for a first program :D

Good

  • Usage of functions
  • Using correct datatypes (dictionary for example)

Some Improvements in random order

  • Read PEP8, the python style guide!

  • fix your indentation

    it is standard to use 4 spaces indentation in python

  • Prettify your code for better readability

    stock={'banana':6,
    'apple':0,
    'orange':32,
    'pear':15,}
    

    this maybe subjective but that is worse to read then this

    stock={
           'banana': 6,
           'apple': 0,
           'orange': 32,
           'pear': 15
          }
    
  • uppercase can be simplified

    def uppercase(x):
        return x[0].upper()+x[1:]
    

    There is a builtin function for this

    >>> print("apple".capitalize())
    Apple
    
  • Remove tripple quotes if they are not neccesary

    name=input('''What is your name?
    ''')
    

    There is no need to let this be over 2 lines, secondly maybe add a space for better UX

    name=input('What is your name? ')
    
  • Use str.format() or f"strings" over old style formatting

    You use old style formatting alot "%s" % "somestring"

    it is better to use the new style formatting "{}".format("somestring")

    See String format best practises

  • Use a if __name__ == "__main__" guard

    It will make your script importable while also being able to run from the CLI

    if __name__ == '__main__':
        name=input('What is your name? ')
        print('Hi, {}, welcome to my fruit store. Here is the menu:'.format(name))
        menu()
        ask_fruit(117)
    
  • Avoid Magic numbers

    money=117
    

    Why 117? Because numbers can't have an explanation it is called a magic number

    Instead you can make it a global

    STARTING_MONEY = 117
    
  • Instead of empty print() use \n

  • Python doesn't really suit itself for recursion (recursion depth restrictions, memory consumption)

    def ask_fruit(money):
    fruit=input('''What fruit do you want?
    ''')
    print()
    if fruit in stock:
      if stock[fruit]>0:
        ask_amount(fruit,money)
      else:
        print('''Sorry, %ss are out of stock
        '''%(fruit))
        ask_fruit(money)
    else:
      print('''Sorry, we don\'t have that, look at the menu.
      ''')
      ask_fruit(money)
    

    Can be rewritten iterative

    def ask_fruit(money):
        while True:
            fruit=input('What fruit do you want? ')
            print()
            if fruit in stock:
                if stock[fruit] > 0:
                    ask_amount(fruit, money)
                else:
                    print('Sorry, {}s are out of stock'.format(fruit))
                    continue
            else:
                print("Sorry, we don't have that, look at the menu.")
                continue
    
\$\endgroup\$
  • \$\begingroup\$ Thanks for the advice, and sorry it took so long for me to reply Changes: -made dictionaries more readable -removed triple quotes where they aren't necessary -used .capitalize() instead of uppercase() -ditched old string formatting for f-string formatting(this is so much better) -used \n instead of print() -used a if name__=='__main' guard -changed ask_fruit function \$\endgroup\$ – zoklev Aug 2 '18 at 5:28
  • \$\begingroup\$ Problems/Questions: -why 4 space indentation? doesn't 2 spaces work perfectly fine as long as you have consistent indentation? -I can't put \n after an input prompt because it makes the input location on the next line, so i have to use print(). Is there a workaround for this? -after using the program more, I realized that the money system doesn't work, after buying some fruit, it doesn't go down. Looking at the code, I don't know what's causing the problem, I wrote "money-=cost" under the condition that the fruit is sold. \$\endgroup\$ – zoklev Aug 2 '18 at 5:28
  • \$\begingroup\$ Problems/Questions pt.2 -I assigned 117 to money because its what you would need to buy everything in the shop, but I agree that it does look weird, normal users would ask "why 117?" But, if not 117, what should I put for money? A random rounded number like 1000? But isn't that also a magic number because there is no reason behind it? -I have way too many functions in my program(it looks confusing) because I didn't know you could put everything in a while loop and repeat it if a condition isn't met. Should I change the whole program so it uses a lot of these while loops instead of functions? \$\endgroup\$ – zoklev Aug 2 '18 at 5:29
  • \$\begingroup\$ Regarding your questions, 1. Because indentations matter alot in Python, the intent is more clear with 4 spaces. 2. The empty print after an input can't be avoided. 3. Any number as starting money would be fine, but the point is you can't "name" a number, so if you assign it to a "named" variable it is much clearer what it is supposed to do. 4. Your functions are fine although they use recursion, you can rewrite those to be iterative for or while loops. \$\endgroup\$ – Ludisposed Aug 2 '18 at 5:52
  • \$\begingroup\$ I tried while loops but the end result turned out incredibly nested and unreadable: repl.it/@zoklev/whileloopclusterfuck. Am I doing something wrong here? \$\endgroup\$ – zoklev Aug 2 '18 at 6:55
2
\$\begingroup\$

Good job!

I would personally prefer to use f-strings, which are more readable.

# from
print('Price: $%s'%(prices[fruit]))
# to
print(f'Price: ${prices[fruit]}')

Using newlines ('\n') would save you from using that many prints and make your code more readable.

Four spaces (instead of two) for indentation would also show your awareness of the PEP8 style conventions

Try/Except structure (catching errors of a particular type) is more preferable and is considered as Pythonic way (refer to https://docs.python-guide.org/).

Use dict.get(). refer to: https://docs.quantifiedcode.com/python-anti-patterns/correctness/not_using_get_to_return_a_default_value_from_a_dictionary.html

\$\endgroup\$
2
\$\begingroup\$

To explain my point above, the only loop you need is in the entry_point. As I've changed the data structure, I thought it best to include some further code examples on the stock handling. This code is missing the make_a_sale function (a combination of your ask_quantity and ask_amount functions).
Don't try to use this code as your program, but take the pieces of logic from it as a way to help your own code.

def menu(stock, money):
    item_length = 0
    for fruit in stock:
        item_length = len(fruit) if len(fruit) > item_length else item_length
    item_length += 1

    print(f"\n{'Fruit':<{item_length}} {'Price':<8} {'Quantity':<5}")

    for name, details in stock.items():
        qty, price = details
        print(f"{name.capitalize():<{item_length}} ${details[price]:<8.2f} {details[qty]:^5}")

    print(f"\nYou have: ${money}")


def sell(stock, selection, quantity, money):
    cost = quantity * stock[selection]["price"]
    confirmation = input(f"Are you sure? That will be ${cost}. [Yes/No] ")
    if "yes" == confirmation.lower():
        money -= cost
        stock[selection]["qty"] -= quantity
        print("Thank you for the business!\n")
        return stock, money


if __name__ == "__main__":
    stock = dict(banana=dict(qty=6, price=4),
                 apple=dict(qty=0, price=2),
                 orange=dict(qty=32, price=1.5),
                 pear=dict(qty=15, price=3),
                 )
    money = 115
    welcome()
    while True:
        menu(stock, money)
        selection, quantity = make_a_sale(stock, money)
        if selection:
            stock, money = sell(stock, selection, quantity, money)

Hope this helps!

\$\endgroup\$
  • \$\begingroup\$ No, sorry, I don't understand that code at all. \$\endgroup\$ – zoklev Aug 3 '18 at 9:30
  • \$\begingroup\$ Sorry, maybe a bit advanced. I just wanted to prove that you only needed a single while loop in the main section (to compare against your updated version). I'm more than happy to clarify some parts that make no sense. \$\endgroup\$ – C. Harley Aug 3 '18 at 15:25

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.