Dragonflies -- Solhee J. and Dehui H.#2
Conversation
ellenjin
changed the title
…_favorites to list comprehension
apradoada
left a comment
apradoada
left a comment
There was a problem hiding this comment.
Overall, really well done!
First off, a couple things y'all did really well:
- Your code is very concise and readable and easy to understand!
- Y'all have some really wonderfully written list comprehensions!
- I loved reading through y'all's analyses of space and time complexity. Overall I think y'all are pretty spot on. When you do start to see complexities that become more like equations, it can be a good sign to either rethink your complexities or your code!
And now just a couple of overall thoughts:
-
When it comes to docstrings and comments, balance is the hardest thing to find. They can be super useful, but they can also add a certain level of clutter, so we have to be super careful. At the end of the day, our code should be pretty self explanatory! We may need a comment here or there to explain sections of your code, and docstrings can be used for more robust functions that require a bit more explanation, but all in all don't be afraid to let your code speak for itself!
-
Don't be afraid to write your own helper functions! Y'all are already at a pretty high level of understanding and skill. When you see functions that work similarly, you are always welcome to pick out the sections that look the same and throw them into their own functions!
Overall, y'all should be very proud of what y'all have done!
| # ******************************************************************************************* | ||
| # ****** Add assertions here to test that the correct movie was added to "watched" ********** | ||
| # ******************************************************************************************* | ||
| assert movie in updated_data['watched'] |
There was a problem hiding this comment.
This Looks good to me! Great idea to check if the whole movie is in the dictionary!
| print(result) | ||
| print(user_data_test) |
There was a problem hiding this comment.
This is a really nice added test! But do make sure to remove any print statements you used for debugging purposes!
| movie = { | ||
| "title": MOVIE_TITLE_1, | ||
| "genre": GENRE_1, | ||
| "rating": RATING_1 | ||
| } |
There was a problem hiding this comment.
It can be so useful to create an object like this for checking equality later on! Great choice!
| if not title or not genre or not rating: | ||
| return None |
There was a problem hiding this comment.
This is such a great pattern to follow in general! The idea to check the negative for our guard clause allows us to continue the regular running of the function!
| return { | ||
| "title": title, | ||
| "genre": genre, | ||
| "rating": rating | ||
| } |
There was a problem hiding this comment.
Tiniest little nitpick here, but traditionally, we'll keep the closing bracket for a dictionary in line with the first line it gets introduced.
| return { | |
| "title": title, | |
| "genre": genre, | |
| "rating": rating | |
| } | |
| return { | |
| "title": title, | |
| "genre": genre, | |
| "rating": rating | |
| } | |
| if not user_data["watched"]: | ||
| return [] | ||
| if not user_data["friends"]: | ||
| return user_data["watched"] |
| for friend in user_data["friends"]: | ||
| if not friend["watched"]: | ||
| continue | ||
| for movie in friend["watched"]: | ||
| if not user_data["watched"] or \ | ||
| (movie not in user_data["watched"] and | ||
| movie not in friends_unique_watched): | ||
| friends_unique_watched.append(movie) | ||
| return friends_unique_watched |
There was a problem hiding this comment.
The same idea as the previous function applies here! You could separate everything out into different sets and find their difference! If you want to get even fancier, you could parse out what it similar between these two functions and create a couple helper functions that can convert the user's "watched" list into a set of titles and the friends "watched" list into a different set of titles as well!
| Parameters: user_data(dict) | ||
| Return: list of reccomended movies(list) | ||
| """ | ||
| movies = get_friends_unique_watched(user_data) |
There was a problem hiding this comment.
Great use of the previous function here!
| Return: list of reccomended movies(list) | ||
| """ | ||
| movies = get_friends_unique_watched(user_data) | ||
| return [movie for movie in movies |
There was a problem hiding this comment.
This is a very well written list comprehension!
| """ | ||
| Get movies from the user's most frequently watched genre which the user has | ||
| not watched, but a friend has watched. | ||
| Parameters: user_data(dict) | ||
| Return: List of reccomended movies(list) | ||
| """ |
There was a problem hiding this comment.
In this case and a couple others, the docstring you've included is actually longer than the function itself! In these cases, think about either shortening the docstring or removing it altogether!
3 participants
No description provided.