I'm looking to improve my code to make it more efficient, such as shortening it but continuing to gain the same output, or to add some more complex functions into the code.

restart ='Y'
while restart not in ('N', 'n', 'NO', 'no'):
    print ("Making A Cup Of Tea")
    num_orders = int(input("How many for Tea? "))
    print ("there are", num_orders, "People for tea")
    orders = []
    for i in range(num_orders):
        b = input ("Person %i, Would you like Sugar? YES/NO " % (i + 1))
        sugar = None
        if b in ("YES", "Y", "y", "yes"):
            sugar = input("How many sugars? ")
            print ("Okay No sugar")

        milk = input("How Much Milk Would You Like? SMALL/MEDIUM/LARGE ")

        print ("Order is being processed, next order:\n")
        orders.append({'sugar': sugar, 'milk': milk })

    print('The orders has been processed with these data:')
    for i in range(num_orders):
        order = orders[i]
        print (' - Person', i + 1, 'wants tea', ('with %i' % int(order['sugar']) if     
        order['sugar'] else 'without'), 'sugar and ', order['milk'], 'milk') 
    restart = input('Would you like to Re-Order? Y/N')
    if restart in ('n', 'N'):
        print ('Okay, Thank You, Enjoy Your Tea')

Some notes:

  • while restart not in ('N', 'n', 'NO', 'no'). In Python is more idiomatic to write while True and break inside the loop. It's not pretty but avoids weird pre-initializations.

  • print ("Making A Cup Of Tea"). No spaces between function name and parenthesis.

  • ('N', 'n', 'NO', 'no'). Use lists for homogeneous values (i.e. same type) and tuples otherwise.

  • print ("there are", num_orders, "People for tea"). Use string.format instead.

  • sugar = None. It's more clear if you write that in an else branch.

  • print (' - Person', i + 1. This line is too long, it's hard to see its components. Break it down.

  • Some bottom-up abstraction is required, use auxiliar functions.

I'd write:

def ask(message, options):
    while True:
        response = input("{0} [{1}]: ".format(message, "/".join(options)))
        possible_options = set(option for idx in range(1, len(response)+1)
            for option in options if response[:idx] == option[:idx])
        if len(possible_options) == 1:
            return list(possible_options)[0]
            print("Unknown option: {0}, try again".format(response))

def ask_yes_no(message):
    return (ask(message, ["yes", "no"]) == "yes")

def get_order(person):
    sugar_question = "Person {0}, Would you like Sugar?".format(person)
    if ask_yes_no(sugar_question):
        sugar = int(input("How many sugars? "))
        print("Okay, No sugar")
        sugar = None

    milk = ask("How Much Milk Would You Like?", ["small", "medium", "large"])
    print("Order is being processed, next order:\n")
    return {"person": person, "sugar": sugar, "milk": milk}

def process_orders():
    while 1:
        print("Making A Cup Of Tea")
        num_orders = int(input("How many for Tea? "))
        print("There are {0} people for tea".format(num_orders))

        orders = [get_order(person) for person in range(1, num_orders+1)]
        print('The orders has been processed with these data:')
        for order in orders:
            print(" ".join([
                ' - Person {0} wants tea'.format(order["person"]),
                ('with {0}'.format(order['sugar']) if order['sugar'] else 'without'), 
                "sugar and {0} milk".format(order["milk"]),

        if not ask_yes_no('Would you like to re-order?'):
            print("\nOkay, Thank You, Enjoy Your Tea")

  • \$\begingroup\$ i read @Roslan answer , i was wondering why not do a conidtion checking instead of while True: , then i scroll down and saw your answer . thank you for the explanation. \$\endgroup\$ – Mourya Mar 20 '14 at 5:11
  • \$\begingroup\$ Your " ".join in the print looks unneeded. \$\endgroup\$ – Veedrac May 24 '14 at 9:13
  • \$\begingroup\$ Doesn't the tuple you're making contain just "homogeneous values"? I think a simpler explanation about why you should use tuples are: for the immutability (irrelevant here), and for the peephole optimisation. \$\endgroup\$ – Peilonrayz Jun 1 '17 at 8:11
  • \$\begingroup\$ @Peilonrayz: Where do I use tuples? ('N', 'n', 'NO', 'no') is the OP's code. More than mutable/immutable I prefer the distinction from the FP world (which is also the math distinction of lists/tuples) \$\endgroup\$ – tokland Jun 1 '17 at 9:42

It's not always about shortening code, but in most cases about having your code easy to maintain. For example, if this code is to go in production, I would change it as follows:

"""A script for ordering a tea party."""

YES = ('y', 'yes')
MILK_SIZES = ('small', 'medium', 'large')

while True:
    print("Making a cup of tea.")
    num_orders = ask_number_of_orders()
    orders = []
    for person_num in range(num_orders):
        print("Person %d, would you like:" % person_num + 1)
        sugar = ask_for_sugar()
        milk = ask_for_milk()
        orders.append((sugar, milk))
        print("Your order is being processed.", end="")
        if person_num + 1 < num_orders:
            print(" Next order:")
    print("The orders have been processed with the following data:")
    for person_num, (sugar, milk) in enumerate(orders):
        order_status = construct_order_status(person_num + 1, sugar, milk)
    restart = input("Would you like to re-order? Y/N.")
    if restart.lower() not in YES:
        print("Ok, thank you, enjoy your day!")

def ask_for_number_of_orders():
    """Get number of orders from the user."""
    while True:
            num_orders = int(input("How many for tea?"))
            if num_order < 1:
                raise ValueError
            print("There are %d people for tea." % num_orders)
            return num_orders
        except ValueError:
            print("Please enter non-negative integer, let's try again.")

def ask_for_sugar():
    """Prompt user for sugar, if yes - how much.

    Returns number of sugar cubes (int) or None.
    while True:
        sugar = input("Would you like sugar? Y/N.")
        if sugar.lower() not in YES:
            print("Okay, no sugar.")
        while True:
                sugar = int(input("How many sugars?"))
                if sugar < 1:
                    raise ValueError
                return sugar
            except ValueError:
                print("Please enter non-negative integer, let's try again.")

def ask_for_milk():
    """Prompts user for the amount of milk."""
    while True:
        milk = input("How much milk would you like? Small/medium/large.")
        if milk.lower() in MILK_SIZES:
            return milk.lower()
            print("Sorry, did not catch that. Small, medium, or large?")

def construct_order_status(person_num, sugar, milk):
    """Constructs order status string.

        person_num: Number of the person.
        sugar: Number of sugar cubes or None.
        milk: Size of the milk: small, medium, or large.

    Returns a string representing order status.
    order_status = " - Person %d wants tea " % person_num
    if sugar is None:
        order_status += "without sugar"
        order_status += "with %d pieces of sugar" % sugar
    order_status += " and %s milk." % milk
    return order_status

I added few user input validators and separated key points of the program into methods. This did not make it shorter, but it did make it more maintainable and easier to change (which is inevitable in a real life software cycle).

I would also add tests, and __main__, but this is going out of the scope of the question. This is, again, just one way to look at the problem: from the real life software development cycle perspective.

EDIT: The only style correction - which is tiny - is to not put space in print (. In python3 it's a function, and according to PEP8 no leading spaces are permitted before the parentheses: http://legacy.python.org/dev/peps/pep-0008/#id18.


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.