The code is sort of an online auction platform and the code returns the last bidder as the winner irrespective of his bid amount

def collections():
    records = { input( ' Name\n ' ) : int( input( 'Bid\n' ) ) }

    def loop_on():
        while input( ' Yes or No\n ' ) == 'Yes':
            details = { input( ' Name\n ' ) : int( input( ' Bid\n ' ) ) }
            records.update( details )

        else:
            return f'The winner is {max( list( records.items() ) )[0]}'
    return loop_on()

print( collections() )

When I run this code I get this error:

---> 10             return f'The winner is {max( list( records.items() ) )[0]}'

TypeError: 'list' object is not callable

I don’t think you need to use list() there.

I think the problem lies in the usage of items() method, which returns the dictionary’s key-value pairs. If you turn it into a list, it will be something like [(“Paul”: 100), (“David”: 200 ), (“Bill”: 150)]. When max() function is applied to this list of tuples without specifying which element of each tuple is to be compared, by default it will be the first one, so it will return the largest name ordered alphabetically instead of the name which has the largest bid value. In this case, it will be “Paul”. If you input somethings like “A”, “B”, “C” as the names in test, it will alway be the last one irrespective of the bid amount.

If you want to find the maximum value in the dictionary records, in fact you can directly use max(records). If you want to get the dictionary key with the max value, use max(records, key=records.get) instead.

Can you use some words to explain your problem and what you’ve tried to fix it?

Its hard to work with you just posting code at people.

please read the whole request first.

You posted code twice and have a very vague problem description in the title.

Please fully describe what problems you are having and what you’ve tried so far.

sure my issue is that I want to convert an object dict_items into a dictionary, by dict( dict.items() ) method it is working but by using { dict.items() } it is showing type error that dict_objects are unhashable.
This is the issue I’m facing .

dict.items() creates a list of tuples, which the { } doesn’t know how to convert into a dictionary. With the { } you have to use the literal key : value syntax.

1 Like
import os

def collects_data():

    records_data = { input( ' Name\n ' ) : int( input( 'Bid\n' ) ) }
    os.system( ' cls ' )
    bid = continued = True

    def starts_loop():

        while bid is continued:
            if input( ' Yes or No\n ' ) == 'Yes':
                updates_data = { input( ' Name\n ' ) : int( input( ' Bid\n ' ) ) }
                os.system( ' cls ' )
                records_data.update( updates_data )

            else:
                winner_name = max( records_data , key = records_data.get )
                winner_bid = max( records_data.values() )
                return f' The winner is {winner_name} with the bid of  { winner_bid } '

    return starts_loop()

print( collects_data() )

By the way the code is completed and now please give your feedback.

Yes, the code can do the basic job of displaying one name of person with the highest bid, provided that unique names and bid values are inputted. But if the code is for practical implementation, there are two more issues for consideration:

  1. If there are more than one person placing the same highest bid, max() function with key=.get method will only return the first entry in the dictionary with the same highest value. Is it what you expect? The code at hand apparently is for single round of sealed bidding. Auction platforms have different tie breaking rules for this case, some may declare the first bidder of the highest bid the winner(while this code may suit this rule), some may randomly pick one among the highest bidders, and some may open further bidding rounds for bidders with the same highest bid.

  2. The code at hand does not block inputs with the same name. If the same name is inputted, update method will replace the old value with the new one in the dictionary, regardless whether the new value is higher or not, so it may lead to cases where certain highest bids cancelled by later inputs, which may be or may not be desirable.

Any thing else particularly in the design and readability in the code?

I think that the point two is not valid as most of the platforms have unique ids which represent the bidders, as using the same names may create confusion and increase complexity.

Stylistically I find two things unusual in the code:

  1. The acution code can be a straightforward one, but it’s written as a nested function. While putting the code within a function can be reasonable if it is intended to be part of a larger system, I think the inner function starts_loop is redundant. It takes no argument, and it’s not called repeatedly within the outer function, apparently the only purpose it serves is to mark the beginning and the end of a while loop. While adding an inner function can still do the job, it introduces an unnecessary layer of complexity. Using comment lines beginning with # should be more perferable if you want to add notes to the code.

  2. Before the while loop two variables are created in this line:

bid = continued = True

Code reviewer have to set an eye on what these two variables are doing, but they are only used in setting the condition of while loop:

while bid is continued:

The two variables are not altered by the prompt ‘Yes or No’ asking whether continue or not, they are not used as loop breaking flags, so they are essentially created to make a while True: line appears more like English. I am not sure it’s a good way of writing code, maybe some others can further comment on it.

Yes, you’re right. In this case the name varable in the code should in fact refer to the unique usernames rather than the real names.

1 Like

The first one is understandable and adoptable also,
The second one seems not much convincing.

I would have also said the second one. Its suspicious to create a variable that you never actually use.

This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.