Maria - Tigers

[mariasilvadev]

No description provided.

[mariasilvadev]

Completed project

[chimerror]

Good Work!

While I point out several minor issues on a variety of topics, this code is functional and generally readable, and thus demonstrates sufficient understanding of the concepts covered. As such, I am marking this Green.

[chimerror]

Minor style preference: I would just delete these skip decorators rather than comment them out. Just helps keep the code cleaner.

There's a few other skip decorators that are like this, but I'll only say it once.

Having talked to other students, they ran into issues trying to delete rather than comment, but I tried it myself and it seemed to work OK.

[chimerror]

As a minor style preference, I recommend just getting rid of commented out code in general unless there's a clear reason to keep it, which should be explained in a comment. This also applies to the comments below where we described how we wanted you to extend the tests, but more just in the sense of keeping code clean.

Deleting these comments would have the additional benefit of keeping the assert on line 126 closer to the other asserts in this function.

I'm sure there's a few other cases where this applies, but I'll only point it out once to avoid repeating myself.

[chimerror]

This test is OK as a quick check, but as a minor test design thing, I would recommend checking all the keys of the movie that got watched, not just the title. The easiest way to do this would likely to save the expected dictionary and use == to compare the entire dictionary, but admittedly, the arrange part of this code that we wrote would have to change...

[chimerror]

Following on the last test design issue, yes, this is a good way to check all keys, not just the title, so good work!

[chimerror]

Looks like this import statement never got used. I know there's a VS Code extension that will add these automatically that often causes it by offering suggestions that create these imports but are most likely not what you want. I would recommend disabling it, Ansel describes how to in this Slack post.

Failing that, make sure to remove these types of statements if they never get used just as a style practice.

[chimerror]

Make sure to take opportunities to reuse earlier waves' code as helper functions. For example, all this code duplicates exactly what was done in get_friends_unique_watched, so it could be used as a helper function:

friends_unique_watched = get_friends_unique_watched(user_data)

recommended_movies = []
for movie in friends_unique_watched:
    if movie["host"] in user_data["subscriptions"]:
        recommended_movies.append(movie)

return recommended_movies

[chimerror]

So minor style nitpick, Python has no requirement that all local variables be defined at the top of the function, so I usually wait until the last possible moment to define them. There are two benefits to this.

First is a chance to avoid unnecessary memory allocation. It doesn't really apply in this case, but if you had checks that made an early return before you actually ended up needing recommended_movies, you can avoid the memory allocation for recommended_movies. Not a big deal for an empty list, but it could be with larger data structures or objects that open other resources such as network connections.

The second is more on the readability side. By defining recommended_movies right when it gets filled in, it more obviously ties recommended_movies with the for loop that fills it. In your current code, it's not too far away, but if you imagine you have more checks of the passed-in values or other code for other tasks, it becomes easier to forget what recommended_movies started as.

[chimerror]

Just like get_available_recs in wave 4, you could have reused get_friends_unique_watched as a helper function and made this code a bit cleaner.

[chimerror]

Though good job at reusing get_most_watched_genre as a helper function here!

[chimerror]

In this case, however, an argument can be made that there might be movies in user_data["favorites"] that are not in user_data["watched"] and thus get_friends_unique_watched can't be reused.

I think it's reasonable to make the assumption that a user would not favorite a movie without watching it, so I think it would have been fine to reuse get_friends_unique_watched here too.

[chimerror]

Though, as a minor style issue, please do use blank lines to separate out "sections" of your code. It can be very hard to follow code without them, and even lead to mistakes in indentation.

You generally did this elsewhere, so I am not sure why not here.