Skip to content

Dragonflies / Bumblebees - Max Iriye / Yekaterina Galochkina C23#4

Open
celestialmage wants to merge 8 commits into
Ada-C23:mainfrom
celestialmage:main
Open

Dragonflies / Bumblebees - Max Iriye / Yekaterina Galochkina C23#4
celestialmage wants to merge 8 commits into
Ada-C23:mainfrom
celestialmage:main

Conversation

@celestialmage
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@mikellewade mikellewade left a comment

Choose a reason for hiding this comment

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

Nice work on getting Viewing Party submitted! I encourage you to read through the comments and heed the ones speaking on keeping your code DRY, variable naming, and data structure selection. I think that focusing on those areas will really take your coding to the next level! Great job!

Comment thread tests/test_wave_01.py
# Assert
assert len(updated_data["watchlist"]) == 0
assert len(updated_data["watched"]) == 1
assert expected_movie in updated_data["watched"]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice work! ✨

Comment thread tests/test_wave_01.py
# Assert
assert len(updated_data["watchlist"]) == 1
assert len(updated_data["watched"]) == 2
assert movie_to_watch in updated_data["watched"]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Comment thread tests/test_wave_03.py

# Assert
assert len(friends_unique_movies) == 3
assert friends_unique_movies.count(INTRIGUE_3) != 2
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍🏿

Comment thread tests/test_wave_05.py
# *********************************************************************
# ****** Complete the Act and Assert Portions of these tests **********
# *********************************************************************
assert len(recommendations) == 0
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Comment thread viewing_party/party.py
Comment on lines +4 to +6
if title and genre and rating:
return {"title": title, "genre": genre, "rating": rating}
return None
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice work on this function, very succinct! A suggestion here is for you to change your guard clause to check for falsy values intitle, genre, and rating` instead. This because the nature of guard clauses to check for edge cases of our function which is our case is our falsy values.

Suggested change
if title and genre and rating:
return {"title": title, "genre": genre, "rating": rating}
return None
if not title and not genre and not rating:
return None
return {"title": title, "genre": genre, "rating": rating}

Comment thread viewing_party/party.py

def get_friends_unique_watched(user_data):

friends_movies = user_data['friends']
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You create this variable but then never use it, is it safe to say that this could be deleted?

Suggested change
friends_movies = user_data['friends']

Comment thread viewing_party/party.py
Comment on lines +101 to +104
for movie in movies:

if movie not in user_data['watched'] and movie not in unique_movies:
unique_movies.append(movie)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Look at the example below:

MOVIE_1 = {
    "title": "Shark Tales",
    "genre": "Comedy",
    "rating": 1
}

MOVIE_2 = {
    "title": "Shark Tales",
    "genre": "Comedy",
    "rating": 3
}

movie_list = [MOVIE_2]

print(MOVIE_1 in movie_list) # False

Your logic here is checking if an entire movie object is in a list. However, if a user and one of their friends has seen the same movie but rated it differently your logic would add it to the unique_movies list. What would you have to change here to ensure that wouldn't be the case? @celestialmage @YekaterinaGalochkina

Comment thread viewing_party/party.py
Comment on lines +111 to +122
def get_available_recs(user_data):

friends_unique_movies = get_friends_unique_watched(user_data)

user_services = user_data["subscriptions"]

rec_movies = []

for movie in friends_unique_movies:
if movie["host"] in user_services:
rec_movies.append(movie)
return rec_movies
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice work on this function!

Comment thread viewing_party/party.py
Comment on lines +131 to +132
favorite_genre = get_most_watched_genre(user_data)
friends_unique_movies = get_friends_unique_watched(user_data)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Keeping your code DRY, love it!

Comment thread viewing_party/party.py
Comment on lines +146 to +147
for movie in user_favorites:
if movie not in friends_movies:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We could also make use of get_unique_watched to get the movies that a user has seen but friends haven't to then loop over them and check if they are in a user's favorite list. This would keep our code DRY as well.

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