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

Scissors_Gloria_Villa #68

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

Scissors_Gloria_Villa #68

wants to merge 2 commits into from

Conversation

ggrossvi
Copy link

Completed, Viewing Party. Sorry, didn't get print statements out.

Copy link

@jmaddox19 jmaddox19 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! I've just left some minor comments below

Comment on lines +1 to +13
from random import randrange

number = randrange(10)

guess = -1
while number != guess:
guess = int(input('Please enter a guess between 0-10 ==>'))
if guess < number:
print(f"{guess} is too low")
elif number > guess:
print(f"{guess} is too high")
else:
print("You guessed it!")

Choose a reason for hiding this comment

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

In the future you can leave extra files like this (and the vscode files) out by just not adding them when you are using git add

Comment on lines +3 to +8
if title == None:
return None
elif genre == None:
return None
elif rating == None:
return None

Choose a reason for hiding this comment

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

This is totally fine, but could be a little more succinct by using or to but all this on one line, since the result is the same in any case.

Copy link
Author

@ggrossvi ggrossvi Mar 31, 2021

Choose a reason for hiding this comment

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

if title == None or genre == None etc...

'genre': genre,
'rating': rating
}
print("movie dictionary:",movie_dict)

Choose a reason for hiding this comment

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

As you noted, its usually best to remove "debugging" print statements like this before submitting.

'''
#the value of `user_data` will be a dictionary \n
# with a key `"watched"`, and a value `[]`
user_data["watched"] = []

Choose a reason for hiding this comment

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

This clears out any items that were already on the watched list. Unfortunately we didn't have a test to catch this issue, but its still good to be on the lookout for bugs like this.

Copy link
Author

Choose a reason for hiding this comment

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

Don't even need line 31. Just wipes out data in this.

Comment on lines +80 to +86
for friend in friends_watched_movie_titles:
if count > 0:
break
if title in friend["watched"]:
count += 1
if count == 0:
user_unique_movies_titles.append(title)

Choose a reason for hiding this comment

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

This totally works, but there are more succinct ways to solve this without the need for a count variable, since it doesn't really matter what the count is if it's above 0.

Suggested change
for friend in friends_watched_movie_titles:
if count > 0:
break
if title in friend["watched"]:
count += 1
if count == 0:
user_unique_movies_titles.append(title)
for friend in friends_watched_movie_titles:
if title not in friend["watched"]:
user_unique_movies_titles.append(title)

Comment on lines +151 to +152
# {"title": "Title A"}
# print(f"{} is unique")

Choose a reason for hiding this comment

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

comments like these are also good to remove before turning in the assignment in the future.

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