Sharks- Kassidy Buslach viewing party submission

[buslakj]

Thanks for looking at my project!

[yangashley]

Hey Kassidy! Great work on your first project! You hit the learning goals and passed the tests 🙂

I left a few comments about renaming variables to be a little more descriptive, ways that you can simplify your code, and called out a time when the return statement was in the for loop when it should have been outside the loop. Let me know if you have any questions about my comments.

Feel free to improve and refactor!

I noticed that you missed adding the assert statements into the test files so your project is currently a 🟡

I'd love to have you update the test files where we ask for additional assert statements to test your code and I can review them. You'll see the call out to add assertions surrounded by lots of *************

April 4 Update: Thanks for adding in the assertions to test your functions, they look good. You've earned a 🟢!

[yangashley]

Nice work adding a guard clause here. You could even put the movie_dict below these lines so that you don't create a dict that you won't use.

Wanted to call out that when we do something like title == None we're literally checking to see if title is equal to None, when what we want to do is check that title is an empty list. We can do that by using not.

Also, you can write this in one line to make the code more concise:

if not title or not genre or not rating:
    return None

[yangashley]

Since you have a guard clause at the beginning of your function if any of title/genre/rating are empty then we'll execute a return statement and leave the function. Since that's the case we don't need to create the dictionary in an else block.

You can also create the dictionary literal in your return statement instead of creating a dict and then adding keys/values. That would look like this:

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

[yangashley]

The requirements in the README wants us to return user_data so we could get rid of line 23 and line 25 here.

Also, removing debugging print statements or test code like on line 20 and 21 keeps your code clean and ensures that unwanted code doesn't accidentally run if somehow those lines got uncommented.

I see you have some print and debugging lines elsewhere so I won't comment on them, but removing those will keep your solution clean.

[yangashley]

We can remove lines 32 and 35 since the requirements asks us to return user_data. If you want to do a check so you're not adding something unexpected if movie is a weird value, then you could write this as:

if movie: 
       user_data["watchlist"].append(movie)
return user_data

This way, we return user_data regardless of whether we added a valid movie or didn't add a weird value that was stored in the movie variable.

[yangashley]

Since we should return user_data, if you wanted to check if title and user_data[" watchlist"] are valid then you would do a couple of things:
-you can combine the two if statements into 1 if title and user_data['watchlist']:
-you can bring the statement return user_data out of the if block. So line 49 should be indented at the same level as line 41 and you'd remove return None

Following my comments for watch_movie, a refactor would look like this:

def watch_movie(user_data, title):
    if title and user_data['watchlist']:
            for movie in user_data['watchlist']: 
                  if movie['title'] == title: 
                    user_data["watched"].append(item)
                    watchlist_data.remove(item)

    return user_data

Happy to chat more about this in our 1:1

[yangashley]

You can delete lines 247, 250-252 since if genre != movie['genre'] nothing will be added to rec_by_genre and you return rec_by_genre at the end.

[yangashley]

Your return statement is actually inside your for loop because it's indented one level deeper (instead of being at the same level as for. This means that when a genre is equal to a movie's genre then it gets added to your list and then the list is returned immediately so your rec_by_genre will only ever have 1 movie in it. With this implementation, your for loop will exit before the next movie gets checked and we might miss some recommended movies.

[yangashley]

Same as the last couple of comments above about creating and returning an empty list - since you have a conditional statement that evaluates before appending a movie, if the condition isn't met then favorite_recs will be empty and you can just return favorite_recs. This way you could remove lines 276-278.

The refactor would look like:

def get_rec_from_favorites(user_data):
    favorite_recs = []
    unique_user_movies = get_unique_watched(user_data)
    user_faves= user_data["favorites"]

    for movies in user_faves:
        if movie in unique_user_movies:
            favorite_recs.append(movie)
    return favorite_recs

[yangashley]

You could consider renaming user_unique to unique_user_movies which is a little more descriptive and implies that this variable is a data structure with more than 1 movie in it.

[yangashley]

Nice work here having the return statement outside of the for loop!

[yangashley]

Nice robust assertion

[yangashley]

Good work writing an entire test here from scratch