Shark - Thuytien Nguyen #114
Conversation
There was a problem hiding this comment.
Great job, Thuytien! One thing I noticed is that you kept your route return statements very consistent, and that's great! Predictability is key, and you did that well!
Some things to consider for the future:
- There was a lot of repeat code when turning a Task into a dictionary. We can turn that into a instance method. There were some other piece of code we could turn into methods as well.
- Some of the helper functions you made can be moved into a new file, like
helper.py.
Other than some duplicate code, your logic was good and your return statements were very consistent!
| # Use corresponding values to create the Task instance. | ||
| post_dict = request.get_json() | ||
| print(post_dict) |
There was a problem hiding this comment.
make sure you get rid of any print statements used for debugging before final submission
| print(post_dict) |
| description = post_dict['description'] | ||
|
|
||
| task = Task(title=title, description=description, completed_at = completed_at) |
There was a problem hiding this comment.
this works just fine! we can also combine these lines, so that we aren't making new variables
| title = post_dict['title'] | |
| description = post_dict['description'] | |
| task = Task(title=title, description=description, completed_at = completed_at) | |
| task = Task( | |
| title=post_dict['title'], | |
| description=post_dict['description'], | |
| completed_at = completed_at | |
| ) |
| if completed_at == None: | ||
| task_dict['is_complete'] = False | ||
| else: | ||
| task_dict['is_complete'] = True | ||
|
|
||
|
|
||
| task_dict['id'] = task.id | ||
| task_dict['title'] = task.title | ||
| task_dict['description'] = task.description | ||
| return_dict = {"task": task_dict} |
There was a problem hiding this comment.
this would make a great instance method in our Task model, like to_json in Solar System or Flasky
| for task in task_db: | ||
| if task.completed_at == None: | ||
| is_complete = False | ||
| else: | ||
| is_complete = True | ||
|
|
||
| task_dict = {'id': task.id, | ||
| 'title': task.title, | ||
| 'description': task.description, | ||
| 'is_complete': is_complete} | ||
| tasks_response.append(task_dict) |
There was a problem hiding this comment.
hmm where have we seen this before? we did it on lines 41-51 as well.
| is_complete = False | ||
| else: | ||
| is_complete = True | ||
|
|
||
| task_dict={} | ||
| task_dict['id'] = task.id | ||
| task_dict['title'] = task.title | ||
| task_dict['description'] = task.description | ||
| task_dict['is_complete'] = is_complete | ||
| if not (task.goal_id == None): | ||
| task_dict['goal_id'] = task.goal_id | ||
| return_dict = {"task": task_dict} |
There was a problem hiding this comment.
we have the same code here again. we should definitely turn this into an instance method inside the Task model
| #----------------------------------- | ||
| @goals_bp.route("/<id>",methods = ["GET"]) | ||
| def get_goal_by_id(id): |
|
|
||
| #------------PUT Method - update goal by id--------- | ||
| @goals_bp.route("/<id>", methods=["PUT"]) |
| return jsonify(return_dict), 200 | ||
| #------------remove goal by id------------ | ||
| @goals_bp.route("/<id>", methods=["DELETE"]) |
| #--------------------------------------------------- | ||
|
|
||
| @goals_bp.route("/<goal_id>/tasks", methods=["POST"]) |
| return jsonify(response_dict), 200 | ||
|
|
||
| @goals_bp.route("/<goal_id>/tasks", methods=["GET"]) |
Ready to submit