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

Sapphire - Sophia & Janice #59

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open

Conversation

sophiat8832
Copy link

No description provided.

Copy link

@audreyandoy audreyandoy left a comment

Choose a reason for hiding this comment

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

Great work Sophia and Janice! You hit all the learning goals for this project and all tests are passing. You have earned a well-deserved 🟢 grade on this project ✨

I added comments, compliments, and hints on ways to refactor your code.

Keep up the great work! ✨

Comment on lines +8 to +9
if not title or not genre or not rating:
return None

Choose a reason for hiding this comment

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

👍 Nice guard clause!

Comment on lines 4 to +15
def create_movie(title, genre, rating):
pass
new_movie = {}

# Check to see if there is a title, genre, rating
if not title or not genre or not rating:
return None
else:
new_movie["title"] = title
new_movie["genre"] = genre
new_movie["rating"] = rating

return new_movie

Choose a reason for hiding this comment

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

We can DRY up this code by returning the dictionary literal.

def create_movie(title, genre, rating):
#   Check to see if there is a title, genre, rating
    if not title or not genre or not rating:
        return None

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

Comment on lines +17 to +25
def add_to_watched(user_data, movie):
# Add movie to user's watched list
user_data["watched"].append(movie)
return user_data

def add_to_watchlist(user_data, movie):
# Add movie to user's watchlist list
user_data["watchlist"].append(movie)
return user_data

Choose a reason for hiding this comment

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

👍 Nice work!

We could also add some input validation (a function that confirms that the data passed into a function is valid (not falsy and the correct data type(s)) can make these functions more robust!

Comment on lines +27 to +34
def watch_movie(user_data, title):
# Check to see if movie's in watchlist, and put into watched
for movie in user_data["watchlist"]:
if movie["title"] == title:
user_data["watchlist"].remove(movie)
user_data["watched"].append(movie)

return user_data

Choose a reason for hiding this comment

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

👍 We can avoid unnecessary iterations by moving the return to the inside of the conditional. The moment that a matching movie is found we'll immediately exit the function. Imagine if the matching movie was found at the start of the list, we can exit the function right away instead of waiting until we've iterated through the entire length of the list.

Comment on lines +39 to +50
def get_watched_avg_rating(user_data):
sum = 0
counter = 0

if not user_data["watched"]:
return 0.0

for movies in user_data["watched"]:
sum += movies["rating"]
counter += 1

return sum/counter

Choose a reason for hiding this comment

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

👍 Nice work this is a really clean solution!

Comment on lines +134 to +144
def get_new_rec_by_genre(user_data):
recommended_by_genre = []
unique_movies = get_friends_unique_watched(user_data)
most_common_genre = get_most_watched_genre(user_data)

# Recommends movies from friends unique list based on most common genre
for movie in unique_movies:
if movie["genre"] == most_common_genre:
recommended_by_genre.append(movie)

return recommended_by_genre

Choose a reason for hiding this comment

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

👍 Aren't helper functions super powerful!

We can also use list comprehension to build recommended_by_genre:

recommended_by_genre = [movie for movie in unique_movies if movie["genre"] == most_common_genre]

Comment on lines +146 to +160
def get_rec_from_favorites(user_data):
friend_movies = []
recommendations = []

# Create list of friends movies
for watched_list in user_data["friends"]:
for movies in watched_list["watched"]:
friend_movies.append(movies)

# Recommend movies in user favorites but not in friends movies
for favorites in user_data["favorites"]:
if favorites not in friend_movies:
recommendations.append(favorites)

return recommendations

Choose a reason for hiding this comment

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

👍 great work!

Comment on lines +56 to +60
recommendations = get_new_rec_by_genre(sonyas_data)

raise Exception("Test needs to be completed.")
#Assert
assert len(recommendations) == 0
# raise Exception("Test needs to be completed.")

Choose a reason for hiding this comment

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

👍

@@ -182,13 +183,14 @@ def test_moves_movie_from_watchlist_to_watched():
# Assert
assert len(updated_data["watchlist"]) == 1
assert len(updated_data["watched"]) == 2
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.

👍

Comment on lines +1 to +14
Pair Plan:
- Access needs
-Tuesday and Thursday cowoorking 1-3
- after class if needed
- work from home if needed -starting 7pm ~ 2 hours

-your learning style
- hands on practice
-how you prefer to receive feedback
- through slack or in person
-team communication skill to improve on with this experience
- tell each other of changes we make on our own (like refactoring)


Choose a reason for hiding this comment

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

❤️ Love the clear communication between you two!

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