Skip to content

tigers_Samiya_Ismail#150

Open
SamiyaOI96 wants to merge 4 commits intoAda-C18:masterfrom
SamiyaOI96:master
Open

tigers_Samiya_Ismail#150
SamiyaOI96 wants to merge 4 commits intoAda-C18:masterfrom
SamiyaOI96:master

Conversation

@SamiyaOI96
Copy link
Copy Markdown

hiii

Copy link
Copy Markdown

@apradoada apradoada left a comment

Choose a reason for hiding this comment

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

Overall, well done, Samiya! The ideas are definitely there, and there are definitely some really sleek pockets of code, they just need to be cleaned up in places. The one major flaw here is the fact that you have not completed all the assertions we asked of you. As a result, I do have to give this code a yellow, which is passing, just with some revisions needed@

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

Choose a reason for hiding this comment

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

The correct assert here would be:

 assert updated_data["watched"][0]["title"] == MOVIE_TITLE_1

The way you currently have it set up checks the entire "watched" list against the movie title.

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

Choose a reason for hiding this comment

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

With the correct assert working, you would also want to check the genre and rating as well! Hint: Simply change the "title" to "genre" and "rating"

Comment thread tests/test_wave_01.py
assert len(updated_data["watched"]) == 2

raise Exception("Test needs to be completed.")
#raise Exception("Test needs to be completed.")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing an assert statement! See if you can assert that the movie_to_watch is in the updated data!

Comment thread tests/test_wave_03.py
assert len(friends_unique_movies) == 3

raise Exception("Test needs to be completed.")
#raise Exception("Test needs to be completed.")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Once we have moved the movies over, we need to assert that INTRIGUE_3, HORROR_1 and FANTASY_4 are all in the friends_unique_movies dictionary.

Comment thread tests/test_wave_05.py
}

raise Exception("Test needs to be completed.")
#raise Exception("Test needs to be completed.")
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're missing an act and assert statement here! Create the recommendations and then make sure the resulting list is empty!

Comment thread viewing_party/party.py
for movie in friends_watched:
if movie["host"] in user_data.get("subscriptions"):
friends_sub.append(movie)
return friends_sub
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This function is pretty much perfect! Great use of your helper functions! The one small tweak I would make is to make variable names that are a little more descriptive of what they are!

Comment thread viewing_party/party.py

def get_new_rec_by_genre(user_data):
user_genre=get_most_watched_genre(user_data)
friends_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.

Great use of helper functions!

Comment thread viewing_party/party.py
friends_movies=get_friends_unique_watched(user_data)
user_movie_rec=[]
if not user_data["watched"]:
return user_movie_rec
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Because we have initialized user_movie_rec as an empty list, this if statement and return statement are redundant. We don't need to worry about whether or not user_data["watched"] is empty!

Comment thread viewing_party/party.py
if not user_data["watched"]:
return user_movie_rec
for movie in friends_movies:
if movie ["genre"]in user_genre:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

user_genre is a string, so an equality operator would be better used here. (== instead of in)

Comment thread viewing_party/party.py
favorite_movie.append(movie)
for movie in user_movies:
if movie in user_movies:
friends_movie_list.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.

The if statements for these two for loops are the exact same condition as what is in the for loop. As a result, everything will actually be added to our lists, so these loops aren't really doing anything. If you combine both loops into one with a condition from each, you'd be good!

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