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

AC2 - Sheung Wan Wong #19

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

Conversation

MelodyW2022
Copy link

No description provided.

# ****** Add assertions here to test that the correct movie was added to "watched" **********
# *******************************************************************************************
assert updated_data["watched"][-1] == {
"title": "It Came from the Stack Trace",

Choose a reason for hiding this comment

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

Good job checking all of the attributes here

@@ -1,23 +1,140 @@
# ------------- WAVE 1 --------------------

def create_movie(title, genre, rating):
pass
if bool(title) and bool(genre) and bool(rating):

Choose a reason for hiding this comment

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

Good job covering validation of both "" and None with "truthiness!"


def watch_movie(user_data, title):

for m_to_watch in user_data["watchlist"]:

Choose a reason for hiding this comment

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

Note, while this may work here, it's considered dangerous practice to modify the content of a data structure that you're looping over. Especially in more complex code, this can open you up to complex errors. A simple alternative here could be to save a copy of what you get from movie in user_data["watchlist"] and iterate over that, so that as you modify user_data["watchlist"], there's no risk.


def get_most_watched_genre(user_data):
from collections import defaultdict
dic = defaultdict(int)

Choose a reason for hiding this comment

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

Was there a reason you'd chosen defaultdict here?

Choose a reason for hiding this comment

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

Nice use of a dict as part of your data processing

if value > max_num:
max_num = value
most_watched_genre = key
return most_watched_genre if user_data["watched"] else None

Choose a reason for hiding this comment

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

Why if user_data["watched"]? Did you mean "if in"? In general I prefer simple return statements, with the more complex logic split up onto previous lines, for added readability and maintainability.



fds_watched_movies_list= []

Choose a reason for hiding this comment

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

Could sets have been an alternative to the nested loops you used in these functions? :)

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