Gricelda Viewing-Party

[GitHub4Gigi]

No description provided.

[yangashley]

In general, we don't want to import values from a test file into our main python files. For our functions in party.py we won't need values from GENRE_1, MOVIE_TITLE_1, RATING_1. We can get our data from the parameters.

[yangashley]

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 an optimization. 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. It shouldn't take very long! You'll see the call out to add assertions surrounded by lots of *************

April 4 Update: Thank you for pushing up your test assertions! You've earned a 🟢

[yangashley]

Nice use of a guard clause here! Since we don't need lines 7-11, you can delete them and make your guard clause the first thing that executes in the create_movie function.

[yangashley]

Your guard clause returns None which will stop the rest of the method from executing when params are not valid. Since that's the case, we can safely assume that lines 15-20 will execute only when there are valid parameters so we can remove else.

[yangashley]

You can directly build your literal dictionary in the return statement here. Taking my comments into account, your refactored method would look like:

def create_movie(title, genre, rating):
    if not title or not genre or not rating:
        return None

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

[yangashley]

When this function runs 'key' evaluates to a dictionary which represents a movie. You could rename 'key' to 'movie' for clarity.

[yangashley]

To express your intent a little more clearly here, you could return 0.0 in a guard clause as the first line of this function.

if not user_data["watched"]:
    return 0.0

Then you can proceed with the rest of your function and you won't need to check if user_data["watched"]: on line 45 before executing the rest of your function

[yangashley]

user_watched is a list and then you iterate through user_data["watched"] and add each movie into this list. To optimize your function you could write:

user_watched = user_data["watched"] without iterating and appending on lines 91 and 93

or you could have your if statement directly access user_data["watched"]. That would look like

if movie not in user_data["watched"] and movie not in total_list:

[yangashley]

Really nicely written and concise!

[yangashley]

I'd recommend using a variable to point to the result of get_most_watched_genre() outside the for loop to help optimize your code like on line 128.

I say this because every time we loop through a friend's movie, we will execute get_most_watched_genre() and this method also does looping. So every iteration now runs another loop (even though we can't tell just by reading if movie["genre"] == get_most_watched_genre(user_data).

Line 130 would look like if movie["genre"] == most_watched_genre:

[yangashley]

You could rename user_genre to something like user_recs_by_genre for clarity

[yangashley]

Could rename user_favorite to something like user_recs_from_faves for clarity