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

Updated to match errors lesson #1

Open
wants to merge 3 commits into
base: solution
Choose a base branch
from

Conversation

audreyandoy
Copy link

  • Uses abort and make_response for 401, and 404
  • Changed self.tricks to return empty list instead of a string to avoid mixing of data types.

@@ -8,12 +8,12 @@ def __init__(self, id, name, breed, tricks=None):
self.name = name
self.breed = breed
if not tricks:
tricks = []
self.tricks = []
Copy link

@kelsey-steven-ada kelsey-steven-ada Apr 21, 2022

Choose a reason for hiding this comment

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

If tricks is None, we'll assign self.tricks to an empty list here, but then immediately overwrite it with None when the line below self.tricks = tricks runs. Maybe we replace all the code for self.tricks assignment with null-coalescing?

self.tricks = tricks or []

Copy link

@anselrognlie anselrognlie left a comment

Choose a reason for hiding this comment

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

The main branch of flasky doesn't look like a total solution. Is there a more appropriate place for this to go?

app/routes/dog_routes.py Outdated Show resolved Hide resolved
app/routes/dog_routes.py Outdated Show resolved Hide resolved
self.tricks = tricks

def to_json(self):
if not self.tricks:
tricks = "No Tricks"
tricks = []

Choose a reason for hiding this comment

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

❤️

@audreyandoy audreyandoy changed the base branch from main to solution April 21, 2022 21:10
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