90pulser/pwp-capstones

pwp-capstones's Issues

Rather than return the message, it may be better to simply print it

pwp-capstones/TomeRater/TomeRater.py

Lines 46 to 50 in bde75d2

	def add_rating(self, rating):
	if (rating >= 0 and rating <= 4):
	self.ratings.append(rating)
	else:
	return "Invalid Rating"

Nice job testing that the rating value is within our accepted value range! However, in the case that the rating is invalid, it may be better to simply print the message "Invalid Rating" instead of using return so that our function's expected return is kept consistent. Without consistent returns, it is hard for a user and/or programmer to know how to handle output from a function. For example, if a function sometimes returns a list and other times returns None, then it will be difficult to know how to handle the output. As such, it is important to ensure that our functions always return only one type (e.g. string, None, list, etc.).

Therefore, instead of sometimes returning an error message and sometimes returning None, we should simply print the error message for the user and have our function always return None:

print("Invalid rating")

Another option would be to return a useful message like Success in the event that the rating is valid. Either option is totally valid!

In any case, this is mostly a suggestion to help ensure we are following Python's best practices; however, the way it is now will still work perfectly!

Don't forget to grab the user and book from the dictionaries in add_book_to_user

pwp-capstones/TomeRater/TomeRater.py

Lines 110 to 120 in bde75d2

	def add_book_to_user(self, book, email, rating = None):
	rating = 0
	if email in self.users:
	User.read_book(book, rating)
	Book.add_rating(rating)
	if book in self.books.keys:
	self.books[book] += 1
	else:
	self.books[book] = 1
	else:
	return "No user with email " + email + "!"

This function is really close to being correct! However, we need to make a couple of small changes to ensure correctness. First, we need to make sure to grab the proper user from our dictionaries; if the user is not extracted from our dictionaries, then none of the operations will be done to the proper user -- we would just be calling the function on an empty User object. We can grab the user from the dictionary like so:

user = self.users[email]

Next, we need to make sure we are not setting a default for the rating (similar to the problem explained here #1). This can be solved by removing the line rating = 0

Lastly, the line self.books.keys needs parentheses because keys is actually a function call. So, the if statement will actually look like so:

if book in self.books.keys():

Putting all of these changes together, we can create a perfect add_book_to_user function:

 def add_book_to_user(self, book, email, rating = None): 
     # Remove the line that set our rating to 0
     if email in self.users: 
         user = self.users[email] # Get the user from the dictionary
         user.read_book(book, rating) # Call our function on the proper User
         book.add_rating(rating) # Call our function on the input Book
         if book in self.books.keys(): # Add parentheses 
             self.books[book] += 1 
         else: 
             self.books[book] = 1 
     else: 
         return "No user with email " + email + "!" 

Overall, this function was really close! It just needed some small modifications to ensure correctness.

Don't forget to report that the ISBN has been change for the book

pwp-capstones/TomeRater/TomeRater.py

Lines 43 to 44 in bde75d2

	def set_isbn(self, new_isbn):
	self.isbn = new_isbn

Great job resetting the ISBN for the book! One small thing: don't forget to report to the successful change to the user so that they know the input has been recorded with the application. This can be done with a simple print statement, such as:

print("Book " + self.title + " now has an ISBN of "+str(self.isbn))

Otherwise, this function is perfect. Nice job!

Make sure to not set the rating as None in the read_book function

pwp-capstones/TomeRater/TomeRater.py

Lines 21 to 23 in bde75d2

	def read_book(self, book, rating):
	rating = none
	self.books[book] = rating

For this function, we don't want to set the rating variable to None because we want to ensure that the rating for the book is kept as the User input it. If we set the rating as None then we will not be able to capture what the User has rated the book they read. So, to capture each rating as input by the User, we should just remove that line and have the function become:

 def read_book(self, book, rating):
     self.books[book] = rating

Now the self.books dictionary will begin holding all the ratings as input by the user.

P.S. The function as it is will actually error out because none is not defined as a keyword in Python. In Python, a "none object" must is spelled with a capital "N" -> None.

Summary

Rubric Score

Criteria 1: Valid Python Code

• Score Level: 2 (Approaches Expectations)
• Comment(s): Some of the functions in the code run with errors. Specifically, the function read_book runs with an error because none is not defined.

Criteria 2: Implementation of Project Requirements

• Score Level: 3 (Meets Expectations)
• Comment(s): The code calls all of the functions in an appropriate order, but it is missing the __repr__ function in the Book class.

Criteria 3: Software Architecture

• Score Level: 4 (Exceeds Expectations)
• Comment(s): The code is separated into distinct classes and functions, each of which are invoked for their own purposes. Good job making sure each function has its own purpose!

Criteria 4: Uses Python Language Features

• Score Level: 2 (Approaches Expectations)
• Comment(s): Python language features are usually used, but there are multiple cases where a coded construction could have been accomplished by a Python built-in. For example, there were times where accessing a value in a dictionary needed to be changed to use Python's built-in features (#8). Similarly, the initialization of the Child classes, Fiction and Non-Fiction, needed to be changed to conform to Python's standards for implementing sub-classes (#5).

Overall Score: 11/16

Many of these functions were really close to being correct! I would highly recommend revisiting the concepts of Classes, inheritance, and how to access values in a dictionary. These topics were covered in other issues (#8 and #5), but Codecademy does a much more comprehensive analysis of these concepts throughout the course. One other small thing: make sure not to set default values for parameters inside our functions (e.g. #1). Setting a default value of a parameter value will make it so our function will not capture the input values as intended by our program.

In any case, most of these functions only required very small changes to be 100% correct. I think reviewing the concepts discussed above and in other issues will be the exact push needed to make this project perfect.

Overall, good job with this project!

In the get_average_rating function, we need to test if the rating is None

pwp-capstones/TomeRater/TomeRater.py

Lines 25 to 29 in bde75d2

	def get_average_rating(self, book):
	rated_average = 0
	for book in self.books:
	rated_average += self.books[book]
	return rated_average / len(self.books)

In the User class, we allow a user to input a None rating for any given book. As such, we have to account for this in our calculation of the average rating. Specifically, if rating is None then we should not count that in our average calculation because that book does not have a rating. This can be achieved by adding an if statement to our for loop, like so:

 def get_average_rating(self, book): 
     rated_average = 0 
     num_books = 0
     for book in self.books: 
         if self.books.get(book):
             rated_average += self.books[book] 
             num_books += 1
     return rated_average / num_books

Notice that we also had to create a new variable num_books in our calculation of the average rating. This is because we should only count a book that has a rating in our average -- if the book does not have a rating, then it should not be included in our count.

It's a very small change, but it makes a big difference in calculation!

No need to set a default for the user_books variable in the add_user function

pwp-capstones/TomeRater/TomeRater.py

Lines 122 to 127 in bde75d2

	def add_user(self, name, email, user_books = None):
	user_books = [None]
	new_user = User(name, email)
	if len(user_books) > 0:
	for book in user_books:
	self.add_book_to_user(book, email)

Here we are really close to a correct function! We just change two things: 1) remove the line that sets a default for the user_books variable, and 2) test that the user_books variable exists instead of checking the length of the list.

We need to remove the line user_books = [None] because this line will overwrite any input that the user has given our function. Note that this is similar to the rating issue explained here.

Next, once that line is removed, we will need to test whether the user_books variable exists or not instead of checking its length. Note that we cannot call the len function on a None object; so, we need to check if the object exists before we can use any function on it.

Putting these two changes together, our new function will look like so:

def add_user(self, name, email, user_books = None): 
     new_user = User(name, email) 
     if user_books: 
         for book in user_books: 
             self.add_book_to_user(book, email) 

Don't forget to initialize the Child classes!

pwp-capstones/TomeRater/TomeRater.py

Lines 65 to 69 in bde75d2

	class Fiction(Book):
	def __init__(Book, author):
	self.title = title
	self.isbn = isbn
	self.author = author

Whenever we are creating a child class, we have to make sure we call the __init__ function of our parent class within the __init__ function of the Child class. This call ensures that our child class has the same functions and variables that are defined in the parent class. There are a couple of ways to do this, but one way that works across all versions of Python is like so:

 class Fiction(Book): 
     def __init__(self, title, author, isbn): # Change to fit the needs of the Fiction class
         Book.__init__(self, title, isbn) # Initialize the Book object
         self.title = title 
         self.isbn = isbn 
         self.author = author

Importantly, notice that the parameters of our Child class' __init__ function has changed. This change not only suits the needs of the Fiction object, but it also satisfies the requirements for initializing the Book object. Specifically, we have changed the parameters from

def __init__(Book, author)

to

def __init__(self, title, author, isbn)

The new __init__ function now contains the necessary parameters for both the Book and Fiction object.

Also, notice how we are initializing the Book class: Book.__init__(self, title, isbn). This call to the __init__ function of the Book class is the line is the line that allows our Fiction object to become a child class of the Book object. With this line, the Fiction class not only has its own functions/variables, but it will now have access to all the functions and variables that are contained within the Book object.

If you have any more questions on inheritance (the concept portrayed above) or how to initialize a child class, I highly recommend revisiting the Codecademy lessons on inheritance!

P.S. Initialization should be the very first thing we do in our __init__ function because it is a best practice. Also, some Python interpreters may be mad if it is not the first thing done in the __init__ function.

P.P.S. This same concept can be applied to the Non-Fiction class. I highly recommend using the above framework to try and fix the initialization of the Non-Fiction class.

Changing how we access values in a dictionary

pwp-capstones/TomeRater/TomeRater.py

Lines 137 to 164 in bde75d2

	def most_read_book(self):
	most_read = ""
	times_read = 0
	for book in self.books:
	if self.books.values[book] > times_read:
	most_read = self.books[book]
	times_read = self.books.values[book]
	return most_read
	def highest_rated_book(self):
	highest_rated = ""
	highest_rating = 0
	for book in self.books:
	if book.get_average_rating[book] > highest_rating:
	highest_rated = book
	highest_rating = book.get_average_rating[book]
	return highest_rated
	def most_positive_user(self):
	positive_user = ""
	positive_score = 0
	for user in self.users:
	for book in User.books:
	if User.books.values[book]/len(User.books) > positive_score:
	positive_score = User.books.values[book]
	positive_user = user
	return positive_user

These functions are all really close to being correct! It performs all the operations and tests we need to determine the highest value in our dictionaries. However, we need to change just a couple of things: how we access values in a dictionary and how certain functions are called.

First, when trying to access a value in a dictionary, we need to change how we access it. Instead of writing self.books.values[book], we need to write self.books[books] to get the proper value.

P.S. values is actually a function that returns a list of all the values contained in the dictionary. For more information on dictionaries, please refer to the Codecademy lessons and/or this documentation: https://realpython.com/python-dicts/

The last thing we have to change is how we are calling the get_average_rating function. We need to change it from book.get_average_rating[book] to book.get_average_rating().

This change is necessary because get_average_rating is a function defined in our Book object. To call this function, we need to access that function from the Book object (e.g. book) and then pass it the necessary parameters. In this case, the function has no parameters, so we can simply call the function like so: book.get_average_rating().

If you have any further questions about calling class functions and/or class concepts, please refer back to the Codecademy lessons.