-
Notifications
You must be signed in to change notification settings - Fork 117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Viewing Party Project #117
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job! I left some comments on your PR for things you did well and things you can refactor. Look at the comments and let me know if you have any questions.
def create_movie(title, genre, rating): | ||
pass | ||
def create_movie(movie_title, genre, rating): | ||
new_movie = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of creating an empty dictionary, you can build a dictionary literal an example could look like
if title and genre and rating:
movie = {
"title": title,
"genre": genre,
"rating": rating
}
return movie
pass | ||
def create_movie(movie_title, genre, rating): | ||
new_movie = {} | ||
if movie_title == None or genre == None or rating == None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here you are supposed to check if this is truthy but this only checks for None. To check if it's truthy you could so something like if not title or not genre or not rating
user_data['watchlist'].append(movie) | ||
return user_data | ||
|
||
def watch_movie(user_data, title): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid iterating and modifying data at the same time, in this case, you could create one for loop with an if conditional that removes the movie from the watchlist and adds the movie to watched in one loop. As soon as we make a change we want to stop iterating. Here is a suggestion:
def watch_movie(user_data, title):
for movie in user_data["watchlist"]:
if movie["title"] == title:
user_data["watchlist"].remove(movie)
user_data["watched"].append(movie)
break
return user_data
count = 0 | ||
for movie in user_data["watchlist"]: | ||
if movie["title"] == title: | ||
add_to_watched(user_data,movie) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great use of helper function
def get_watched_avg_rating(user_data): | ||
sum = 0 | ||
for i in range (0,len(user_data["watched"])): | ||
print(user_data["watched"][i]["rating"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove print statements after debugging/testing. Print statements should only be added if it related to the user experience
genre_count = genre_counts.get(genre,0) | ||
genre_counts[genre] = genre_count + 1 | ||
counter +=1 | ||
max_genre = max(genre_counts, key=genre_counts.get) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great use of the max method here. If you didn't use the max method what would you do in its place?
for i in range(len(user_data['watched'])): | ||
watched_list.append(user_data["watched"][i]) | ||
for i in range(len(user_data['friends'])): | ||
for movie in range(len(user_data['friends'][i]["watched"])): | ||
friends_watched.append(user_data['friends'][i]["watched"][movie]) | ||
for movie in friends_watched: | ||
if movie not in watched_list: | ||
list_difference.append(movie) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these lines of code are similar to lines 67-77. This would be a good candidate for a helper function.
# ----------------------------------------- | ||
# ------------- WAVE 4 -------------------- | ||
# ----------------------------------------- | ||
|
||
def get_available_recs(user_data): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙌🏽🙌🏽
# ----------------------------------------- | ||
# ------------- WAVE 5 -------------------- | ||
# ----------------------------------------- | ||
def get_new_rec_by_genre(user_data): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙌🏽🙌🏽
|
||
def get_rec_from_favorites(user_data): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of doing two for loops you could do something like
def get_rec_from_favorites(user_data):
movies = get_unique_watched(user_data)
recs = []
for movie in movies:
if movie in user_data['favorites']:
recs.append(movie)
return recs
Sea Turtles
Katina Carranza