Skip to content

Spruce - Symone B.#95

Open
syblades wants to merge 3 commits intoAda-C16:masterfrom
syblades:master
Open

Spruce - Symone B.#95
syblades wants to merge 3 commits intoAda-C16:masterfrom
syblades:master

Conversation

@syblades
Copy link

No description provided.

Copy link
Collaborator

@audreyandoy audreyandoy left a comment

Choose a reason for hiding this comment

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

Great work Symone! You hit all the learning goals for the project and passed all the tests. You definitely earned a 🟢 on this project 🌲✨

I added comments primarily on ways to refactor your code. In the future, I'd like to see more docstrings in your functions, especially where you are using functions/methods in Python that are not covered in our curriculum. Docstrings are a great way of leaving notes to your future self on your approach to writing these functions.

Feel free to refactor and improve on this project and do let me know if you want me to take another look for fun 😄

Keep up the great work!

Comment on lines 88 to 100
def get_friends_unique_watched(user_data):
unique_friends_watched = []
user_watched = []

for movie in user_data["watched"]:
user_watched.append(movie)

for friend in user_data["friends"]:
for movie in friend["watched"]:
if movie not in user_watched:
if movie not in unique_friends_watched:
unique_friends_watched.append(movie)
return unique_friends_watched
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
def get_friends_unique_watched(user_data):
unique_friends_watched = []
user_watched = []
for movie in user_data["watched"]:
user_watched.append(movie)
for friend in user_data["friends"]:
for movie in friend["watched"]:
if movie not in user_watched:
if movie not in unique_friends_watched:
unique_friends_watched.append(movie)
return unique_friends_watched
def get_friends_unique_watched(user_data):
unique_friends_watched = []
user_watched = []
for movie in user_data["watched"]:
user_watched.append(movie["title"])
for friend in user_data["friends"]:
for movie in friend["watched"]:
if movie["title"] not in user_watched:
if movie not in unique_friends_watched:
unique_friends_watched.append(movie)
return unique_friends_watched

The movie dictionaries change between wave 3 and wave 4 so consistently keeping track of the titles instead will find the unique movies.

@@ -63,7 +63,7 @@ Verify that you're in a python3 virtual environment by running:
- `$ python --version` should output a Python 3 version
Copy link
Collaborator

Choose a reason for hiding this comment

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

This README file didn't need to be pushed. You can use commands like git add <filename> to ensure that you're only adding specific files to be tracked, committed, and pushed to Github.

Comment on lines +2 to +11
def create_movie(title, genre, rating):
if bool(title) and bool(genre) and bool(rating):
new_movie = {
"title": title,
"genre": genre,
"rating": rating
}
return new_movie
else:
return None
Copy link
Collaborator

Choose a reason for hiding this comment

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

To check for the truthiness of a variable in an expression we can use its name directly instead of using bool.

Suggested change
def create_movie(title, genre, rating):
if bool(title) and bool(genre) and bool(rating):
new_movie = {
"title": title,
"genre": genre,
"rating": rating
}
return new_movie
else:
return None
def create_movie(title, genre, rating):
if title and genre and rating:
new_movie = {
"title": title,
"genre": genre,
"rating": rating
}
return new_movie
else:
return None

We can further improve on this logic by reversing the conditional and check for a 'failed' case early, allowing the 'main' logic to be unindented and have more of a focus in the function body. This is known as a guard clause and is a common pattern used in other languages like c, go, etc.

Suggested change
def create_movie(title, genre, rating):
if bool(title) and bool(genre) and bool(rating):
new_movie = {
"title": title,
"genre": genre,
"rating": rating
}
return new_movie
else:
return None
def create_movie(title, genre, rating):
if not title or not genre or not rating:
return None
new_movie = {
"title": title,
"genre": genre,
"rating": rating
}
return new_movie

__




def add_to_watched(user_data, movie):
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍




def add_to_watchlist(user_data, movie):
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍



#WAVE 3
def get_unique_watched(user_data):
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍




def get_friends_unique_watched(user_data):
Copy link
Collaborator

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
Collaborator

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
Collaborator

Choose a reason for hiding this comment

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

👍

if movie not in friends_watched:
if movie not in user_recs_from_favs:
user_recs_from_favs.append(movie)
return user_recs_from_favs
Copy link
Collaborator

Choose a reason for hiding this comment

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

💡 Quick-Tip: To make your project neater and follow pep8 guidelines, you can have VS Code autoformat your files by right-clicking on a file in VS Code and select Format Document With... then select python from the drop-down menu. You'll then be prompted to install autopep8, click yes and the package will be installed within your virtual environment.

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.

3 participants

Comments