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

C-17 Otters - Marlyn Lopez #114

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

Conversation

marlopez24
Copy link

No description provided.

Copy link

@kendallatada kendallatada left a comment

Choose a reason for hiding this comment

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

Hi Marlyn! Your submission has been scored as green. Please check your code to see the comments I left. Let me know if you have any questions. Nice work! :)

def create_movie(title, genre, rating):
pass
result = None

Choose a reason for hiding this comment

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

This variable isn't being used. Moving forward, be sure to check your code for unnecessary work. :)

def create_movie(title, genre, rating):
pass
result = None
if not title or not genre or not rating:

Choose a reason for hiding this comment

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

Nice validation

result = None
if not title or not genre or not rating:
return None
movie = {}

Choose a reason for hiding this comment

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

Small suggestion to cut down on lines of code - rather than creating the dictionary in steps, you could do it all in one step like this:
movie = {"title": title, "genre": genre, "rating": rating}
or even just returning it without creating a variable:
return {"title": title, "genre": genre, "rating": rating}



def watch_movie(user_data, title):
for key in user_data["watchlist"]:

Choose a reason for hiding this comment

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

Consider using a more descriptive variable name for key. Also, pay attention to how many times your loop will iterate and when it exits. Right now, your loop will keep going until it reaches the end of watchlist even if it already found the title. This is a suuuuper easy mistake to make. Now, that we have chatted about Big O, pay attention to if your code is doing unnecessary work.

assert len(updated_data["watched"]) is 2
assert len(updated_data["watchlist"]) == 1
assert len(updated_data["watched"]) == 2
# assert updated_data["watched"][0]["title"] == MOVIE_TITLE_1

Choose a reason for hiding this comment

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

good asserts 👍



def friends_movies(user_data):

Choose a reason for hiding this comment

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

Love that you're creating helper functions ❤️

friends_movie_titles.append(movie)
return friends_movie_titles

def get_friends_unique_watched(user_data):

Choose a reason for hiding this comment

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

This is also a great solution! I would challenge you to see if you can get this solution down to 2 for loops as well.

# -----------------------------------------
# ------------- WAVE 4 --------------------
# -----------------------------------------

def get_available_recs(user_data):
recommendations = []
for movie in get_friends_unique_watched(user_data):

Choose a reason for hiding this comment

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

Loooove that you're taking advantage of functions you have already written. ❤️ Now that we have talked about Big O, think about how the efficiency of the helper function impacts the time complexity of the overall solution. Is the helper function doing work that is unnecessary for this solution? Think about how you could refactor your code so it does less work.

# -----------------------------------------
# ------------- WAVE 5 --------------------
# -----------------------------------------

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.

Beautiful solution ✨


def get_rec_from_favorites(user_data):
favorites_recommendation = []
new_movie = get_unique_watched(user_data)

Choose a reason for hiding this comment

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

This is a super clean solution! I would challenge you to think about how you can refactor this code to improve the time complexity. Consider if it would be faster to not use the get_unique_watched() function.

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