Unexpected Python result to Code

Question

dictionary = {'Jason Seifer': ['Ruby Foundations', 'Ruby on Rails Forms', 'Technology Foundations'], 'Kenneth Love': ['Python Basics', 'Python Collections'], 'Daniel': ['Python Basics', 'Python Collections', 'test', 'test']}
def most_class(dictionary):
 current_count = 0
 max_count = 0
 best_teacher = "none"
 for key in dictionary:
 for values in dictionary.values():
 current_count += 1
 if current_count > max_count:
 max_count = current_count
 best_teacher = key
 return best_teacher

The goal is to determine which key in the dictionary has the most values. The answer should be Daniel, but I am getting a different answer each time I run the code (presumably due to pythons hashing of the dictionary.

Can anyone explain why I am getting this result, and how it can be fixed?

Answer 1

You have two problems.

1. You need to reset current_count each time around, not just at the start.
2. dictionary.values() gives you the list of lists of books, not the list of books for that author.

To solve the second one, I would use for author, books in dictionary.items(): rather than for key in dictionary:. Then you won’t have to use dictionary[key] each time. After making these changes, we have:

def most_class(dictionary):
 max_count = 0
 best_teacher = "none"
 for author, books in dictionary.items():
 current_count = 0
 for book in books:
 current_count += 1
 if current_count > max_count:
 max_count = current_count
 best_teacher = author
 return best_teacher

Further, I might pull the if out of the loop:

def most_class(dictionary):
 max_count = 0
 best_teacher = "none"
 for author, books in dictionary.items():
 current_count = 0
 for book in books:
 current_count += 1
 if current_count > max_count:
 max_count = current_count
 best_teacher = author
 return best_teacher

At that point, it becomes clear that we don’t need a for loop at all, and can just use len:

def most_class(dictionary):
 max_count = 0
 best_teacher = "none"
 for author, books in dictionary.items():
 if len(books) > max_count:
 max_count = len(books)
 best_teacher = author
 return best_teacher

Your code might also benefit from using the max function as others have suggested, although I would probably use it a different way:

def most_class(dictionary):
 return max(dictionary.items(), default=('none', 0),
 key=lambda item: len(item[1]))[0]

• user3839821: Thakns for this answer, I believe it finally works. I am still learning python, so I am not sure what you did in line 4. Could you explain what is going on? I would have thought that would result in a syntax error, is python assuming you are picking the values in a key for each of the dictionary items? Thanks.
• icktoofay: @user3839821: I’m not sure if you’ve learned tuples yet, but they look like this: (1, 2, 3). Iterating over the return value of items, you get tuples of (key, value). It turns out Python has a handy ‘unpacking’ syntax, where if you have for a, b in it:, that’s roughly equivalent to for tup in it: a = tup[0]; b = tup[1]. So by doing that, we’re iterating over all of the keys and values in the dictionary simultaneously, putting each key in author and its associated value in books.

Answer 2

You're iterating over the dictionary more times than you need to, and making this much more difficult than it needs to be. You can do it much more simply by combining the Python built-in max() with comprehensions. Try this instead:

dictionary = {
 'Jason Seifer': ['Ruby Foundations', 'Ruby on Rails Forms',
 'Technology Foundations'], 
 'Kenneth Love': ['Python Basics', 'Python Collections'], 
 'Daniel': ['Python Basics', 'Python Collections', 'test', 'test']}
# get maximum number of items any one person has
maximum_len = len(max(dictionary.values(), key=len))
# build a list of everyone with that number of items
most = [name for name, classes in dictionary.items() 
 if len(classes) == maximum_len]
print(most)

This code prints:

['Daniel']

Not only does this replace loops with comprehensions, in the event of a tie, you get a list of the names instead of just the first one found. For example, if we change the dictionary definition to this:

dictionary = {
 'Jason Seifer': ['Ruby Foundations', 'Ruby on Rails Forms', 
 'Technology Foundations', 'other_item'], 
 'Kenneth Love': ['Python Basics', 'Python Collections'], 
 'Daniel': ['Python Basics', 'Python Collections', 'test', 'test']}

Then this will be printed instead:

['Daniel', 'Jason Seifer']

• user3839821: Thanks. I am unsure how I could use this within my code, do you mind being a bit more verbose? Similarly, I am still unsure why my code does NOT work, even with the process being more difficult.
• mattsilver: Your code doesn't work because you should have for values in dictionary[key] I think. Either way, this answer is on the right track. You definitely don't need for loops for this

Answer 3

You have to know how to use dictionary , then I recommend to do:

dictionary = {'Jason Seifer': ['Ruby Foundations', 'Ruby on Rails Forms', 'Technology Foundations'], 'Kenneth Love': ['Python Basics', 'Python Collections'], 'Daniel': ['Python Basics', 'Python Collections', 'test', 'test']}
def most_class(dictionary):
 best_teacher = None
 class_of_best_teacher = 0
 for teacher_name in dictionary.keys():
 class_of_teacher = len(dictionary[teacher_name])
 if class_of_teacher > class_of_best_teacher :
 best_teacher = teacher_name
 class_of_best_teacher = class_of_teacher
 return best_teacher
print most_class(dictionary)

Answer 4

you iterates through keys in the dictionary but you for got to reset your current_count back to zero between each key. So current_count and max_count counts up to 9 together and best_teacher will be the last key that the for loop iterates through.

Fix your code like this.

...
for key in dictionary:
 current_count = 0
 ...

It work but I would like to do this instead:

best_teacher = sorted(dictionary.keys(), key=lambda x: len(dictionary[x]))[-1]

Answer 5

1. reset current_count for each key
2. should use dictionary[key] instead of dictionary.values()

the following code return Daniel.

dictionary = {'Jason Seifer': ['Ruby Foundations', 'Ruby on Rails Forms', 'Technology Foundations'], 'Kenneth Love': ['Python Basics', 'Python Collections'], 'Daniel': ['Python Basics', 'Python Collections', 'test', 'test']}
def most_class(dictionary):
 current_count = 0
 max_count = 0
 best_teacher = "none"
 for key in dictionary:
 current_count = 0
 for values in dictionary[key]:
 current_count += 1
 if current_count > max_count:
 max_count = current_count
 best_teacher = key
 return best_teacher