Scissors - Araceli M. #61
Conversation
aracelim1234
changed the title
There was a problem hiding this comment.
Overall, the code is well-written and readable. You demonstrated that you can write code that can work well with lists and dictionaries. You also gave yourself an opportunity to practice writing code to handle exception handling!
For the most part the code to handle nested data structures looks great as well, but there is one bug related to nested data structures in get_rec_from_favorites. See my comment below for more details.
There are also several instances where I wonder if you forgot the check what the README said the function was meant to do and instead relied solely on the unit tests. This demonstrates that you have gotten really good at reading unit tests already, which is great! However, the goal is that the functions are written to satisfy the requirements in the README and pass the tests.
|
|
||
| if len(user_data["watched"]) == 0: | ||
| user_data["watched"] = user_data["watchlist"] | ||
| user_data["watchlist"] = [] |
There was a problem hiding this comment.
This will introduce a bug when the watchlist contains more than one item because it will completely clear the list every time.
|
|
||
| def watch_movie(user_data, movie): | ||
|
|
||
| movie_to_watch = { |
There was a problem hiding this comment.
The intention is that the movie we are trying to watch is passed in through the movie parameter.
There is not a situation where we ask that the function should "watch" a hardcoded movie like this.
| user_data["watchlist"].append(movie) | ||
| return user_data | ||
|
|
||
| def watch_movie(user_data, movie): |
There was a problem hiding this comment.
It looks like this function was cleverly written to pass the tests, but doesn't solve the requirements listed in the README.
I'm curious to talk with you about what your process for solving this was.
| try: | ||
| average_rating = sum(rating_list) / len(rating_list) | ||
| except ZeroDivisionError: | ||
| return 0.0 |
| elif user_data["watched"][x] in user_data["friends"][0]["watched"]: | ||
| continue | ||
| elif user_data["watched"][x] in user_data["friends"][1]["watched"]: | ||
| continue |
There was a problem hiding this comment.
This works for the tests we provided, but only because the tests don't cover a case where the user has 1 friend or more than 2 friends.
In the future, in addition to passing the tests, I encourage you to think of what other cases might exist and if you can write code to handle those additional cases as well.
| movie_list = [] | ||
| friends_unique_movie_list = [] | ||
|
|
||
| for i in range(len(user_data["friends"])): |
There was a problem hiding this comment.
This is a great example of writing code that can handle any number of friends! (in contrast with the code in get_unique_watched above.
| else: | ||
| continue |
There was a problem hiding this comment.
There's nothing wrong with having these two lines here but I want to note that the function would do exactly the same thing without them
| if user_data["watched"]: | ||
| for i in range(len(user_data["friends"])): | ||
| for j in range(len(user_data["friends"][i]["watched"])): | ||
| if user_data["friends"][i]["watched"][j]["genre"] == "Intrigue": |
There was a problem hiding this comment.
This is another example where the code only works because of how limited the test cases are.
The requirement in the README is that the genre should match the user's most frequently watched genre, not that it should always be "Intrigue"
| for k in range(len(user_data["friends"][j])): | ||
| if user_data["favorites"][i] in user_data["friends"][k]["watched"]: |
There was a problem hiding this comment.
There is some funkiness happening here that the tests would catch if they were more thorough.
user_data["friends"][j] is a hash so this loop is running a certain number of times based on how many key/value pairs are in that hash, which doesn't seem to be what the code is intended to do.
There was a problem hiding this comment.
This confusion generally happens less when writing loops without indexes. For example, for friend in user_data["friends"]
| else: | ||
| continue | ||
|
|
||
| for x in movie_list: |
There was a problem hiding this comment.
In cases like this, I'd recommend naming variables like this as movie to be more clear and meaningful.
No description provided.