Skip to content
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

Viewing Party Project #117

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Viewing Party Project #117

wants to merge 2 commits into from

Conversation

kmcarranza
Copy link

Sea Turtles
Katina Carranza

Copy link

@tgoslee tgoslee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job! I left some comments on your PR for things you did well and things you can refactor. Look at the comments and let me know if you have any questions.

def create_movie(title, genre, rating):
pass
def create_movie(movie_title, genre, rating):
new_movie = {}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of creating an empty dictionary, you can build a dictionary literal an example could look like

if title and genre and rating:
        movie =  {
            "title": title,
            "genre": genre,
            "rating": rating
            }
        return movie

pass
def create_movie(movie_title, genre, rating):
new_movie = {}
if movie_title == None or genre == None or rating == None:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you are supposed to check if this is truthy but this only checks for None. To check if it's truthy you could so something like if not title or not genre or not rating

user_data['watchlist'].append(movie)
return user_data

def watch_movie(user_data, title):
Copy link

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

count = 0
for movie in user_data["watchlist"]:
if movie["title"] == title:
add_to_watched(user_data,movie)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great use of helper function

def get_watched_avg_rating(user_data):
sum = 0
for i in range (0,len(user_data["watched"])):
print(user_data["watched"][i]["rating"])
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove print statements after debugging/testing. Print statements should only be added if it related to the user experience

genre_count = genre_counts.get(genre,0)
genre_counts[genre] = genre_count + 1
counter +=1
max_genre = max(genre_counts, key=genre_counts.get)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great use of the max method here. If you didn't use the max method what would you do in its place?

Comment on lines +85 to +92
for i in range(len(user_data['watched'])):
watched_list.append(user_data["watched"][i])
for i in range(len(user_data['friends'])):
for movie in range(len(user_data['friends'][i]["watched"])):
friends_watched.append(user_data['friends'][i]["watched"][movie])
for movie in friends_watched:
if movie not in watched_list:
list_difference.append(movie)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these lines of code are similar to lines 67-77. This would be a good candidate for a helper function.

# -----------------------------------------
# ------------- WAVE 4 --------------------
# -----------------------------------------

def get_available_recs(user_data):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙌🏽🙌🏽

# -----------------------------------------
# ------------- WAVE 5 --------------------
# -----------------------------------------
def get_new_rec_by_genre(user_data):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙌🏽🙌🏽


def get_rec_from_favorites(user_data):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of doing two for loops you could do something like

def get_rec_from_favorites(user_data):

    movies = get_unique_watched(user_data)
    recs = []

    for movie in movies:
        if movie in user_data['favorites']:
            recs.append(movie)

    return recs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants