Viewing Party - Reid M.

[reidhowdy]

No description provided.

[anselrognlie]

Project one done!

All the waves are complete and all tests are passing! I was able to follow your code, and it looks like you were trying out a lot of different approaches as you progressed through the waves!

The main thing I would be careful of is interpreting the test data as narrowly as you did for a few cases. For finding the most frequent genre, we shouldn't assume that the values in the test data are the only possible values. Then, for the two unique watched functions, we should assume that the only key present in the movies is the title. Check out my detailed feedback in those functions.

When iterating through these nested data structures, also try not to overcomplicate things. If we know the key in a dictionary we are interested in, we can get its value directly without introducing another layer of iteration. More details inside!

But overall, nice job.

[anselrognlie]

In Python, we generally avoid explicit line wrapping (using \) and prefer implicit line wrapping (any expression surrounded by () can be wrapped pretty freely). But here, we could avoid wrapping entirely by simplifying the condition here. With the style you have, the condition can be written as:

    if title and genre and rating:

We can take advantage of the truthiness of non-empty strings to make this work.

Another approach for this function is to invert the sense of the condition, and bail out early with the required None result. Setting up our logic like this is called a guard clause or a guard condition. It lets us trivially handle special cases, returning as quickly as possible so that we don't need to worry about them during the remainder of the function. In the example below, notice that this also lets us un-indent the main logic, letting it be more of the focus of the function.

def create_movie(title, genre, rating):
    """Takes 3 values, returns a dictionary."""

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

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

[anselrognlie]

👍

[anselrognlie]

👍

[anselrognlie]

It's a little dangerous to modify a collection while we are iterating over it (here, the watchlist). In this case, it ends up being OK, since we really only expect a single copy of a movie to be in the watchlist at a time. However, if we were iterating over a list, and there was the possibility of multiple adjacent items being in the list that we want to remove, we could end up skipping over items as the list items get shifted around underneath the iteration.

If we know for certain that we only need to remove one thing, we could break out of the loop as soon as we've removed the one thing. Alternatively, we could find the movie record, and then wait to perform the remove until after completing that loop.

Again, nothing goes wrong in the current situation, but in general, we should be wary of modifying a collection while we are iterating through it.

[anselrognlie]

A more pythonic way to check whether a list is empty is:

    if not user_data["watched"]:

[anselrognlie]

The main difference between my previous feedback and here is that if we want to make sure the friend output list is unique. We would need to track the friend titles we've added to the output list, something like this:

    unique_output_titles = set()
    for friends_watched_lists in user_data["friends"]:
        for indiv_dict in friends_watched_lists["watched"]:
            if indiv_dict["title"] not in unique_output_titles and indiv_dict["title"] not in user_watched_set:
                only_friends_watched.append(indiv_dict["title"])
                unique_output_titles.add(indiv_dict["title"])

[anselrognlie]

Consider using a set rather than a list here since you're using this to store the result titles for lookup purposes. On line 178, the in lookup for lists is O(n), but the in lookup for sets is O(1).

[anselrognlie]

With some changes to the how we use parentheses here, we can avoid the explicit line wrapping, like:

Suggested change

	if (watched_dict["host"] in user_data["subscriptions"]) \
	and (watched_dict not in rec_list) \
	and (watched_dict["title"] in friends_watched_list_titles):
	if (watched_dict["host"] in user_data["subscriptions"]
	and watched_dict not in rec_list
	and watched_dict["title"] in friends_watched_list_titles):

[anselrognlie]

Don't we already have a function that calculates a unique list of movies watched by the friends. What if we used that, and found the movies that are in the user's favorite genre?

[anselrognlie]

Here again, we could simplify the looping as in get_unique_watched. No need for the .values() call.