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

Maple - Sabrina Lauredan #82

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

Conversation

SabrinaLauredan
Copy link

No description provided.

Copy link

@spitsfire spitsfire left a comment

Choose a reason for hiding this comment

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

I know you didn't finish every way, Sabrina, but you still did a great job! I added a few suggestions on how to use less memory space by combining for loops and variables together in one.

Maybe if you have time over the break, you can come back to this and finish up the last few tests you have left!

@@ -0,0 +1,129 @@
#wave 1

def create_movie(title, genre, rating):

Choose a reason for hiding this comment

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

👍

Comment on lines 4 to 9
if title == None:
return None
if genre == None:
return None
if rating == None:
return None

Choose a reason for hiding this comment

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

since all of these if statements will produce the same return statement, let's combine them!

Suggested change
if title == None:
return None
if genre == None:
return None
if rating == None:
return None
if title == None or genre == None or rating == None:
return None

movie = {"title": title, "genre": genre, "rating": rating}
return movie

def add_to_watched(user_data, movie):

Choose a reason for hiding this comment

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

👍

user_data['watched'].append(movie)
return user_data

def add_to_watchlist(user_data, movie):

Choose a reason for hiding this comment

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

👍

Comment on lines 27 to 28
if user_data['watched'] == []:
user_data["watched"].append(movie)

Choose a reason for hiding this comment

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

so, if user_data["watched"] is empty, no worries! it will still append the appropriate movie from the watchlist to the watched list on line 26. So we don't need this extra checking if statement:

Suggested change
if user_data['watched'] == []:
user_data["watched"].append(movie)

unique_movies = []
for friend in user_data["friends"]:
for movie in friend["watched"]:
if not is_movie_in_list(user_data["watched"], movie) and not is_movie_in_list(unique_movies, movie):

Choose a reason for hiding this comment

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

👍 I see a helper function! noice!


# wave 4

def get_available_recs(user_data):

Choose a reason for hiding this comment

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

👍


# #wave 5 pt1

def get_new_rec_by_genre(user_data):

Choose a reason for hiding this comment

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

👍


# wave 5 pt 2

def get_rec_from_favorites(user_data):

Choose a reason for hiding this comment

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

👍


# helper

def is_movie_in_list(list, movie):

Choose a reason for hiding this comment

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

👍

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.

3 participants