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

Scissors - Mai Truong #55

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

truongmt89
Copy link

No description provided.

Copy link

@jmaddox19 jmaddox19 left a comment

Choose a reason for hiding this comment

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

Great work! I just have a few very small comments for potential improvements and questions.

@@ -0,0 +1,15 @@
{

Choose a reason for hiding this comment

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

In the future if you use git add main.py instead of git add . you can leave files like this out of your commits.


def create_movie(movie_title, genre, rating):
# create dictionary
new_movies = {

Choose a reason for hiding this comment

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

Since this is only one movie, it'd be more appropriately named new_movie

Comment on lines +122 to +123
# if len(user_data["watched"]) == 0:
# return []

Choose a reason for hiding this comment

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

In the future, your submissions can be a little cleaner by removing old code like this

most_watched[movie["genre"]] = 1
else:
most_watched[movie["genre"]] += 1
popular_genre = max(most_watched, key = most_watched.get)

Choose a reason for hiding this comment

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

Because of the indentation, popular_genre is currently being calculated at each iteration of the loop even though you don't need it until the end.

Suggested change
popular_genre = max(most_watched, key = most_watched.get)
popular_genre = max(most_watched, key = most_watched.get)

Comment on lines +143 to +179
def get_rec_from_favorites(user_data):

user_favories = user_data["favorites"]
friends_movies = user_data["friends"]
favorites = []
user_rec = []

for movie in user_favories:
favorites.append(movie)

for movie in favorites:
for movie in friends_movies["watched"]:
# if movie not in friends_movies["watched"]:
user_rec.append(movie)

return user_rec

def get_rec_from_favorites(user_data):

user_favories = user_data["favorites"]
friends_movies = user_data["friends"]
favorites = []
friend_watched = []
user_rec = []

for movie in user_favories:
favorites.append(movie)

for friend in friends_movies:
for movie in friend["watched"]:
friend_watched.append(movie)

for movies in favorites:
if movies not in friend_watched:
user_rec.append(movies)

return user_rec

Choose a reason for hiding this comment

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

I'm curious why this same function is defined twice.
In this case the first function definition will just get overwritten by the 2nd one.

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