-
Notifications
You must be signed in to change notification settings - Fork 117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sea Turtles - San Robinson #115
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job San! I left some comments on what you did well and what you can refactor. Let me know if you have any questions.
from enum import unique | ||
from logging.handlers import RotatingFileHandler | ||
from re import L, U | ||
import re | ||
from tests.test_constants import MOVIE_TITLE_1, USER_DATA_2 | ||
import copy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you using all of these imports? If not, then go ahead and remove them.
user_data["watchlist"].append(movie) | ||
return user_data | ||
|
||
def watch_movie(user_data, title): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid iterating and modifying data at the same time, in this case, you could create one for loop with an if conditional that removes the movie from the watchlist and adds the movie to watched in one loop. As soon as we make a change we want to stop iterating. Here is a suggestion:
def watch_movie(user_data, title):
for movie in user_data["watchlist"]:
if movie["title"] == title:
user_data["watchlist"].remove(movie)
user_data["watched"].append(movie)
break
return user_data
user_data["watched"].append(movie_to_watch) | ||
return user_data | ||
|
||
def watched_movie(user_data, title): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that you created this helper function but you aren't using it. What were your plans to use it?
user_data["watched"].append(movie_to_watch) | ||
return user_data | ||
|
||
def nonexistent_movie(user_data, title): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that you created this helper function but you aren't using it. What were your plans to use it?
# ----------------------------------------- | ||
# ------------- WAVE 2 -------------------- | ||
# ----------------------------------------- | ||
|
||
def get_watched_avg_rating(user_data): | ||
avg_rating_list= [] | ||
# check_empty = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can remove this if you aren't using it
# get list of all friends movies(titles) | ||
# check if each users movie is in list of movie(title) | ||
|
||
def get_unique_watched(user_data): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you have similar lines of code in get_friends_unique_watched
that would be a good candidate for helper function.
# ----------------------------------------- | ||
# ------------- WAVE 4 -------------------- | ||
# ----------------------------------------- | ||
|
||
def get_available_recs(user_data): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙌🏽
mode_genre = get_most_watched_genre(user_data) | ||
new_recommendations = [] | ||
rec_recommendations = get_friends_unique_watched(user_data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great use of helper functions here
|
||
|
||
|
||
#refactored code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would move this commented code to your notes or a txt file for your reference. You wouldn't want this to be in your final code.
get_most_watched = [] | ||
if bool(user_data["watched"]) == False: | ||
get_most_watched.append(None) | ||
popular_genre = statistics.mode(get_most_watched) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when using a method/library add a short comment on how this works on your code for readability.
No description provided.