4
\$\begingroup\$

In relation to this question, an upcoming exam (on the 20th May 2019) requires me to carry out this task using Visual Basic, Python or Pascal/Delphi. I am using Python (version 2.7/3.7) since the program is easier to read and write in this language. Here is the pre-release for my exam:

An auction company has an interactive auction board at their salerooms, which allows buyers to place bids at any time during the auction. Before the auction starts, the sellers place their items in the saleroom with a unique number attached to each item (item number). The following details about each item need to be set up on the interactive auction board system: the item number, number of bids, description, and reserve price. The number of bids is initially set to zero.

During the auction, buyers can look at the items in the saleroom and then place a bid on the interactive auction board at the saleroom. Each buyer is given a unique number for identification (buyer number). All the buyer needs to do is enter their buyer number, the item number, and their bid. Their bid must be greater than any existing bids.

At the end of the auction, the company checks all the items and marks those that have bids greater than the reserve as sold. Any items sold will incur a fee of 10% of the final bid to be paid to the auction company.

Write and test a program or programs for the auction company.

• Your program or programs must include appropriate prompts for the entry of data, data must be validated on entry.

• Error messages and other output need to be set out clearly and understandably.

• All variables, constants, and other identifiers must have meaningful names.

You will need to complete these three tasks. Each task must be fully tested.

Task 1 – Auction set up.

For every item in the auction the item number, description, and the reserve price should be recorded. The number of bids is set to zero. There must be at least 10 items in the auction.

Task 2 – Buyer bids.

A buyer should be able to find an item and view the item number, description, and the current highest bid. A buyer can then enter their buyer number and bid, which must be higher than any previously recorded bids. Every time a new bid is recorded the number of bids for that item is increased by one. Buyers can bid for an item many times and they can bid for many items.

Task 3 – At the end of the auction.

Using the results from TASK 2, identify items that have reached their reserve price, mark them as sold, calculate 10% of the final bid as the auction company fee and add this to the total fee for all sold items. Display this total fee. Display the item number and final bid for all the items with bids that have not reached their reserve price. Display the item number of any items that have received no bids. Display the number of items sold, the number of items that did not meet the reserve price and the number of items with no bids.

Here is my solution (program) to the question, task by task:

number_of_bids = 0
count = 0
auction_fee = 0.0
item_no = 0
sold_items = 0
less_than_reserve = 0
zero_bids_items = 0
min_items = 10
while True:
    try:
        n = int(input("Enter the number of items in the auction: "))
        if n < min_items:
            raise ValueError
    except ValueError:
        print ("Number of items have to be at least 10!")
    else:
        current_highest_bid = [0.0]*n
        item_bids = [0]*n
        item_description = []*n
        reserve_price = []*n
        item_numbers = []*n
        buyer_no_Array = [0]*n
        break

#TASK 1

for i in range(n):
    item_no = item_no + 1
    item_numbers.append(item_no)
    print ("ENTER DETAILS FOR ITEM NO.", item_no)
    description = input("Enter description for item no. " + str(item_no))
    item_description.append(description)
    reserve = float(input("Enter reserve price for item no. " + str(item_no)))
    reserve_price.append(reserve)

#TASK 2

while count != "y":
    for i in range(n):
        print ("Item number:", item_numbers[i], "Description:", item_description[i], "Reserve price:", reserve_price[i], end = " ")
        print ("Current highest bid:", current_highest_bid[i], "No. of bids:", item_bids[i], end = " ") 
        print ("Buyer with highest bid:", buyer_no_Array[i])   

    choice = input("Do you want to bid for items in this auction? (y/n): ")

    if (choice == "y"):

        buyer_no = int(input("Enter your buyer ID: "))

        item_choice = int(input("Enter the item number for your choice of item: "))
        if item_choice in item_numbers:
            index = item_numbers.index(item_choice)    
            while True:
                try:
                    bid_price = float(input("Please enter your bid: "))
                    if (bid_price <= current_highest_bid[index]):
                        raise ValueError
                except ValueError:
                    print ("Bid should be higher than the current highest bid!")
                else:
                    current_highest_bid[index] = bid_price
                    number_of_bids = int(item_bids[index]) + 1
                    item_bids[index] = number_of_bids
                    print ("Bids for", item_description[index], "are:", item_bids[index])
                    buyer_no_Array[index] = buyer_no
                    break
        else: 
            print ("Invalid item code!") 
    elif (choice == "n"):
        count = input("END THE AUCTION? Enter 'n' to continue bidding or 'y' to end the auction: ")

#TASK 3

if (count == "y"):
    sold = False
    for i in range(len(current_highest_bid)):
        if (current_highest_bid[i] >= reserve_price[i]):
            sold = True
            sold_items = sold_items + 1
            auction_fee = auction_fee + (current_highest_bid[i] * 0.1)
    print ("The total auction company fee is: $", auction_fee)

    for i in range(len(current_highest_bid)):
        if (current_highest_bid[i] < reserve_price[i]):
            less_than_reserve = less_than_reserve + 1
            print ("Item code", item_numbers[i], "with final bid $", current_highest_bid[i], "has not reached the reserve price.")

    for i in range(len(current_highest_bid)):
        if (current_highest_bid[i] == 0):
            zero_bids_items = zero_bids_items + 1
            print ("Item code", item_numbers[i], "has not received any bids.")

    print ("Number of items sold are:", sold_items)
    print ("Number of items that did not meet the reserve price are:" , less_than_reserve)
    print ("Number of items with no bids are:", zero_bids_items)
else:
    print ("Invalid input!")

NOTE: I would recommend you to test the program by using a smaller value of min_items so that it is easier to test the program.

So, I would like to know whether I could make this code shorter and more efficient (to achieve the best marks), as my exam board focuses more on the efficiency of a program.

\$\endgroup\$
  • 3
    \$\begingroup\$ Are you sure this works? []*n does not create a list of length n, you need an element to repeat, so probably [0]*n or [""]*n. \$\endgroup\$ – Graipher May 12 at 12:56
  • 2
    \$\begingroup\$ No this is just done to create a list of length n, so that you cannot append any more values to the list. \$\endgroup\$ – Justin May 12 at 12:59
  • 2
    \$\begingroup\$ But it doesn't matter. You can remove the *n's for the empty lists but leave the ones which have 0's in them. \$\endgroup\$ – Justin May 12 at 13:01
  • 2
    \$\begingroup\$ I need item_description, reserve_price and item_numbers to be empty lists as I am supposed to append values to the list. \$\endgroup\$ – Justin May 12 at 13:05
3
\$\begingroup\$

Performance and efficiency are not the main points that you should care about at this stage, especially for this simple task. Since your code will be scored by other persons, you should aim for readability and clarity. There is an official Style Guide for Python Code that especially Python novices should try to adapt in order to write good-looking Python code. I will try to link to relevant parts of the style guide where appropriate. With this in mind, let's have a look at your code.

Data structures

Keeping data consistent that is spread out over several list can be tedious, as you may have experienced yourself while writing the code. In your case, you will have to work with 6 (! - if I did not misscount) parallel lists, e.g. when placing a bid. A far more convenient approach would be to have a single collection of auction items, where each item has all the fields you have spread out over those lists at the moment. Depending on your level of Python experience, you may write a class for that, or use some of Python's builtin data structures. For the moment, I will stick with the second alternative.

So, what do we need? Each item has the following properties:

  • item description
  • reserve price
  • current highest bid
  • number of bids
  • buyer with highest bid

as well as an item number (why this is separate will hopefully become clearer in a few moments).

Python offers several possible solutions for that:

  1. have a tuple/list where each element can be accessed by an index, and you have to know which index represents which part of the information
  2. use a dictionary where the name of the properties above is used as keys

I'ld tend to use the second approach in your case, just because it's more explicit to use and it's harder to make misstakes. With this in our mind, lets look at how such an "item" might look like as a Python dict:

item = {"description": "", "reserve price": 0.0, 
        "current highest bid": 0.0, "buyer with highest bid": None,
        "number of bids": 0}

As you can see, the properties can be used verbatim as keys in the dicitionary which makes it as straightforward as item["number of bids"] += 1 to update the number of bids placed on an item. All the other properties may be used in the same fashion.

So 5 of 6 down, one to go: item number. I chose to exclude the item number from the list above, since it's role is a little bit special here. Why? Because the item number is used to identify the item. If you'ld like to stick to simple, consecutive item numbers, the easiest way is to put a bunch of these dictionaries in a list and use the position in the list as implicit item number.

items = []
for i in range(n):
    items.append({
        "description": "",
        "reserve price": 0.0,
        "current highest bid": 0.0,
        "buyer with highest bid": None,
        "number of bids": 0
    })

You could also use a list comprehension for this1. Since we're now in a situtation where the position in the list matters and identifies the element, it might be wise to convert the list into a tuple after its creation: items = tuple(items). Python tuples are not mutable, which means for your purpose you are not allowed to add and remove items. The dictionaries, which are elements of the tuple, can still be modified.

You basically can go full berserk from here on. You want non-consecutive item numbers? Use a dict with the item numbers as keys and the corresponding item dicts as values. Include the bid validation into the items themselves? Write a class with methods.


All the feedback below refers to your original code, but can easily adapted to the new data structure above. Some of it will even become obsolete.


Handling user input

There a various part where you have to handle user input. Although you have tagged your question with Python 2 and Python 3, I sincerly hope you are actually using Python 3. Otherwise you are setting yourself up for some trouble because using input(...) in Python 2 is "risky", to put it mildly. So let's assume Python 3 from here on.

The part where you're asking for the number of items is relatively robust, and even includes some input validation, which is good. However, it would be best to wrap it into a function and separate it from the list initialization. That would lead to something like:

def get_number_of_items(min_items):
    """Get the number of items offered during the auction"""
    while True:
        try:
            n = int(input("Enter the number of items in the auction: "))
            if n < min_items:
                raise ValueError
        except ValueError:
            print (f"Number of items have to be at least {min_items}!")
        else:
            return n

Apart from wrapping the code into a function, two other things have happened here. First, the function got a short docstring. Second, the error message now includes the actual value of min_items and not just a fixed value of 10. To read more about the type of string formatting that's happening here, go and look for "Python 3 f-strings", e.g. at the Python doc or this blog post.

On the second instance of user input, you took way less care to validate the input. There are no checks in place to actually make sure that a description is given, or that the reserve price is actually a non-negative number. To be fair, the task does not explicitely state that, but I think it's fair to assume that the reserve price is at least 0. You can also take advantage of string formatting when creating the prompts, so use description = input(f"Enter description for item no. {item_no}") instead of description = input("Enter description for item no. " + str(item_no)). Since this is for you to learn something and you already have an example for a function that performs a similar task, I leave the implementation as an exercise to you.

String formatting

String formatting was alread mentioned above, but I just want to point out, that basically all of your calls to print(...) with dynamic output should be refactored to use it. Example:

print(f"Item number: {item_numbers[i]} "
      f"Description: {item_description[i]} "
      f"Reserve price: {reserve_price[i]} "
      f"Current highest bid: {current_highest_bid[i]} "
      f"No. of bids: {item_bids[i]} "
      f"Buyer with highest bid: {buyer_numbers[i]}")

The example also makes use of Python's implicit line joining within function parenthesis. It would also be possible to put + in front of all the strings apart from the first one to make it clearer that these strings are supposed to be joined.

Avoid globals

After you have packed everything into nice, single purpose functions, reduced the variable clutter, and the myriad of lists, it's now time to cut down the amout of global variables in your script. Global variables are all variables outside of functions at the script's top-level. You should try to avoid them whenever possible since they leak into functions and more unexpected things. A best-practice often found in Python code is to define a final main() function that uses the other functions and implements the scripts actual functionality. So the high-level code structure could now look something like:

def get_number_of_items(min_items):
    """Get the number of items offered during the auction"""
    ...


# other functions
...


def main():
    """Hold the auction"""
    min_items = 10
    ...
    n = get_number_of_items(min_items)
    items = []
    for i in range(n):
        ...

    # Task 1
    ...

    # Task 2
    ...

    # Task 3
    ...


if __name__ == "__main__":
    main()

The only thing that's new here is if __name___ == "__main__":, which is Python's way of telling "this part (main()) is only run if the file is used as a script".

Miscellaneous

What follows is a loose collection of minor bits and pieces, that are not as severe as the aspects I have already talked about. You might consider them as "good to know".

There are some parts of the code which seem to be overly complicated and/or "non-Pythonic", like

number_of_bids = int(item_bids[index]) + 1
item_bids[index] = number_of_bids

which could be expressed as simple as

item_bids[index] += 1

Another instance of "overcomplicated" code is

for i in range(n):
    item_no = item_no + 1
    ...

I would consider

for i in range(n):
    item_no = i + 1
    ...

as a clearer alternative, since this is a more direct way to see that you are actually asigning consecutive item IDs in a loop.

As others have already told you

item_description = []*n
reserve_price = []*n
item_numbers = []*n

does not create empty lists of length n. In your code this is no problem since you simply append to these lists until they have length n, so you should just work with

item_description = []
reserve_price = []
item_numbers = []

When performing the final evaluation described in Task 3, I would strongly recommend to define those "summary" variables like sold_items, auction_fee, and so on, just there where they are actually used and not at the start of the script. Especially if you have followed the initial advice on the data structure and the final evaluation might also easily be put into its own function.


1 A word of warning: Although it might be tempting to use items = [item for i in range(n)], be aware that this would not lead to the same result! In that case, all elements of items whould point to a single object in memory and altering any element, e.g. items[0] whould also alter items[1] and so forth.

\$\endgroup\$
  • \$\begingroup\$ Upvoted! I haven't met a single tutor or my own teacher who could have helped me fulfill my task. Amazing code review :) Thanks a million for your time and patience. \$\endgroup\$ – Justin May 17 at 2:44

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.