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

Jessica E Ambriz-Madrigal - Lions #162

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

Conversation

Jessicaenvisions
Copy link

Want to look over wave 4. Finished until wave 3.

Copy link

@goeunpark goeunpark left a comment

Choose a reason for hiding this comment

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

Great job, Jessica!

Your functions and conditionals look polished, your assert statements are clear, and nested loops iterate beautifully! I left a few suggestions inline. Remember that you can also write more than one commit message per wave! Based on our revised requirements, this project is a Green. 🟢

Comment on lines +122 to +124
assert updated_data["watched"][0]["title"] == MOVIE_TITLE_1
assert updated_data["watched"][0]["genre"] == GENRE_1
assert updated_data["watched"][0]["rating"] == RATING_1

Choose a reason for hiding this comment

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

Wonderful assertion statements! ✨

@@ -143,25 +150,33 @@ def test_moves_movie_from_watchlist_to_watched():
assert len(updated_data["watchlist"]) == 1
assert len(updated_data["watched"]) == 2

raise Exception("Test needs to be completed.")
assert updated_data["watched"][1] == movie_to_watch

Choose a reason for hiding this comment

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

Nice! 👍🏻

Comment on lines +58 to +60
assert INTRIGUE_3 in friends_unique_movies
assert HORROR_1 in friends_unique_movies
assert FANTASY_4 in friends_unique_movies

Choose a reason for hiding this comment

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

Wonderful! 💯

@@ -1,23 +1,144 @@
# ------------- WAVE 1 --------------------

from distutils.file_util import move_file
from pickle import FALSE

Choose a reason for hiding this comment

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

Remember to clear any import statements that we don't need before submission! 😉

def create_movie(title, genre, rating):
pass
new_movie = {}
if title == None or genre == None or rating == None:

Choose a reason for hiding this comment

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

Great chained logic in this conditional!

Comment on lines +59 to +60
average_rating = all_ratings_added/len(user_data["watched"])
return average_rating

Choose a reason for hiding this comment

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

Great job combining division with the length calculation!

Comment on lines +70 to +71
maximum_key = None
maximum_value = 0

Choose a reason for hiding this comment

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

Hmm, could we have more descriptive variable names? Maybe best_genre_name and best_genre_count?


# -----------------------------------------
# ------------- WAVE 3 --------------------
# -----------------------------------------


def 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.

Small nit/preference: I like to initialize all my known variables right after defining the function so I can keep track of them easier! For example, I (personally, meaning you don't have to do this!) would put L88 and L93 right after each other around L88.

Comment on lines +128 to +135
for friend in user_data["friends"]:
for movie in friend["watched"]:
for host in movie["host"]:
if movie["title"] not in movie_list:
if movie not in friends_movies:
if host in movie_list:
friends_movies.append(movie)

Choose a reason for hiding this comment

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

👀 OOOO, interested in where you might be going here! Let me know if you want to walk through this Wave in our 1:1s!

Comment on lines +101 to +103
movie_list = []
for movie in user_data["watched"]:
movie_list.append(movie["title"])

Choose a reason for hiding this comment

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

We can get rid of this and refer to user_data["watched"] directly in L108.

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