arnoldna / tomerater Goto Github PK

Rubric Score

Criteria 1: Valid Python Code

• Score Level: 4
• Comment(s): The code that you provide runs without errors.

Criteria 2: Implementation of Project Requirements

• Score Level: 3
• Comment(s): You provide correct implementations of the methods and classes required by the assignment. Besides the points that I mention in my feedback, your implementations were correct.

Criteria 3: Software Architecture

• Score Level: 3
• Comment(s): Code is separated into distinct classes and methods, each which are invoked for their own purposes. The only drawback is the point I make in my feedback on your TomeRater class about repeated (i.e. copy/paste) code.

Criteria 4: Uses Python Language Features

• Score Level: 3
• Comment(s): Although there are a few places where more use of Python built-in behaviors or functions would have saved you time, you make enough use of Python built-in functionality to aid in accomplishing your tasks.

Overall Score: 13/16

Thank you for your submission. I've given feedback on some aspects of your code here. I hope that you find in it something useful.

Thanks for your submission. The points that I make below mostly concern small errors that effect complete correctness of some methods.

A minor comment. The methods change_email and __repr__ should print instead of return the strings.

Two comments on your get_average_rating implementation for the User class. First, let's consider the following snippet.

  for key, value in self.books.items():
    if value is not None:
      sum_of_ratings.append(value)
  for value in sum_of_ratings:
    total += value

Here you can combine the two for loops into one:

  for key, value in self.books.items():
    if value is not None:
      sum_of_ratings.append(value)
      total += value

From this we see that we don't need to make sum_of_ratings hold the values. It is enough for it to simply keep track of the number of values which are not None. Keeping the name the same, we can accomplish that like this

sum_of_ratings = 0
...
  for key, value in self.books.items():
    if value is not None:
      sum_of_ratings += 1
      total += value

Now we can divide by these values directly. That brings me to my second comment. Let's look at the following line.

  return round(total/sum_of_ratings, 1) # changed from "return round(total / len(sum_of_ratings), 1)"

One thing that you should check here is that sum_of_ratings != 0 to avoid a division by zero error. It may be the case that all ratings are in fact None in which case both total and sum_of_ratings will be zero.

The same comments I make for the User class's get_average_rating method also goes for the same method in the Book class.

Within the print_catalog and print_user methods, there's no need to place the print statement within brackets. You can simply write a usual for-in loop. That is,

for Y in X:
  print(Y)

As written now, as a byproduct of using list comprehension to print the entries, you create a list of None values.

Notice that the methods, most_read_book, highest_rated_book, and most_positive_user have implementations which are copy/paste of one another with minor changes. Whenever you have a circumstance where a core functionality being performed is the same across many parts of your code, it is often a good idea to think of abstracting that core functionality out into a separate function. You can then call that function multiple times instead of copying code multiple times. The same can be said about get_n_most_read_books and get_n_most_prolific_readers.