Skip to content
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 -Shannon Bellemore - Sea Turtles #104

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

eggoweggo
Copy link

No description provided.

Comment on lines +8 to +13
if title == None or genre == None or rating == None:
return None
movie_info['title'] = title
movie_info['genre'] = genre
movie_info['rating'] = rating
return movie_info
Copy link

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

Suggested change
if title == None or genre == None or rating == None:
return None
movie_info['title'] = title
movie_info['genre'] = genre
movie_info['rating'] = rating
return movie_info
if title and genre and rating:
movie = {
"title": title,
"genre": genre,
"rating": rating
}
return movie

pass
# create movie information
movie_info = {}
if title == None or genre == None or rating == None:
Copy link

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

return user_data


def watch_movie(user_data, title):
Copy link

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


# -----------------------------------------
# ------------- WAVE 2 --------------------
# -----------------------------------------
def get_watched_avg_rating(user_data):
combined_rating = 0
if user_data["watched"] == []:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you don't need to compare an empty list here. The pythonic way would be if not user_data["watched"]

# calculate average of rating
for movie in user_data["watched"]:
combined_rating += movie["rating"]
average = (combined_rating/len(user_data["watched"]))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest moving this out of the loop so you are only doing the average once.

# use mode function to find genre mode of mode list
for movie in user_data["watched"]:
mode_list.append(movie["genre"])
return statistics.mode(mode_list)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job looking up different methods to use. I would also push you to think of your own implementation and get familiar with explaining how this method works under the hood.


# -----------------------------------------
# ------------- WAVE 3 --------------------
# -----------------------------------------
def get_unique_watched(user_data):
# make list of friends movies
friend_movies = []
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a good approach, to make your code even more efficient you could make friend_movies equal to a set() instead of an empty list. Then on line 71 you would add it to the set instead of appending it to the list.

user_movies.append(movie["title"])
# if movie not in users movie list, add to friend unique movies
friend_unique_movies = {}
for friend in user_data["friends"]:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an experienced developer, it took a few times to read through this loop and understand what was happening. Adding a comment explaining how the code is actually working inside of this for loop will make it easier for others to understand.

Comment on lines +111 to +112
user_most_watched_genre = get_most_watched_genre(user_data)
friend_unique_movies = get_friends_unique_watched(user_data)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great job using those helper functions!

Comment on lines +124 to +131
favorite_movies = []
for movie in user_data["favorites"]:
favorite_movies.append(movie)
# iterate over favorite movies and check if has not been watched by friends
movie_from_favorites = []
for movie in favorite_movies:
if movie in user_unique_movies:
movie_from_favorites.append(movie)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
favorite_movies = []
for movie in user_data["favorites"]:
favorite_movies.append(movie)
# iterate over favorite movies and check if has not been watched by friends
movie_from_favorites = []
for movie in favorite_movies:
if movie in user_unique_movies:
movie_from_favorites.append(movie)
for movie in movies:
if movie in user_data['favorites']:
recs.append(movie)

@tgoslee
Copy link

tgoslee commented Mar 28, 2022

Overall great job on your first project Shannon! I added a few suggestions on how you can refactor some of your code to be more efficient. Take a look and let me know if you have any questions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants