Pull Request #87
Pull Request #87
Conversation
kaidamasaki
left a comment
kaidamasaki
left a comment
There was a problem hiding this comment.
Great job!
One thing you should focus on is readability, especially with things like indenting your dictionaries.
Other than that your code looks quite good! (Though you should make sure that if you do something clever like with select_model and you're working with a partner you comment it enough for them to understand.)
Well done!
| response_body = jsonify({"goal" : { | ||
| "id": self.get_id(), | ||
| "title": self.title, | ||
| }}), response_code |
There was a problem hiding this comment.
Style: indentation.
| response_body = jsonify({"goal" : { | |
| "id": self.get_id(), | |
| "title": self.title, | |
| }}), response_code | |
| response_body = jsonify({"goal" : { | |
| "id": self.get_id(), | |
| "title": self.title, | |
| }}), response_code |
| # try: response_body = jsonify({"task" : { | ||
| # "id": self.get_id(), | ||
| # "goal_id": self.goal_id, | ||
| # "title": self.title, | ||
| # "description": self.description, | ||
| # "is_complete": self.is_complete(), | ||
| # }}) | ||
| # except AttributeError: |
There was a problem hiding this comment.
Style: clean up commented out code.
| try: | ||
| int(number) | ||
| except: | ||
| return make_response(f"{number} is not an int!", 400) |
There was a problem hiding this comment.
This is cleaner if you just use abort:
| return make_response(f"{number} is not an int!", 400) | |
| abort(make_response(f"{number} is not an int!", 400)) |
(abort raises a special exception that results in a response being sent back immediately.)
| def model_select(url): | ||
| if "goals" in url: | ||
| mdl = Goal | ||
| else: | ||
| mdl = Task | ||
| return mdl |
There was a problem hiding this comment.
This is bordering on too clever but does really DRY your code up!
(By too clever I mean it might be hard to debug/understand.)
| task = Task.query.get(task_id) | ||
| task.completed_at = datetime.now(timezone.utc) | ||
| db.session.commit() | ||
| print(response.text) |
There was a problem hiding this comment.
Style: clean up debugging print calls.
2 participants
No description provided.