Zoisite Luwam Ghebremicael #121
Conversation
completes get_one/get_all POST/GET functions
completes DELETE one task function
passing wave 1
passing wave2
passing test_mark_complete
passing mark_incomplete
wave5 2 tests passing
passing all waves
| from .task_routes import tasks_bp | ||
| app.register_blueprint(tasks_bp) | ||
|
|
||
| from .goal_routes import goals_bp | ||
| app.register_blueprint(goals_bp) |
There was a problem hiding this comment.
Nice choice to split up the routes into files that are more specific to the resources they work with!
Based on the paths to the files, it looks like the route files are directly in the app folder. Since we have more than one route file, I'd suggest creating a routes directory to organize those route files in.
| def validate_task(id): | ||
| try: | ||
| id = int(id) | ||
| except: | ||
| abort(make_response({"message": f"Task {id} is invalid"}, 400)) | ||
|
|
||
| task = Task.query.get(id) | ||
|
|
||
| if not task: | ||
| abort(make_response({"message": f"Task {id} not found"}, 404)) | ||
|
|
||
| return task | ||
|
|
||
|
|
||
| def validate_goal(id): | ||
| try: | ||
| id = int(id) | ||
| except: | ||
| abort(make_response({"message": f"Goal {id} is invalid"}, 400)) | ||
|
|
||
| goal = Goal.query.get(id) | ||
|
|
||
| if not goal: | ||
| abort(make_response({"message": f"Goal {id} not found"}, 404)) | ||
|
|
||
| return goal |
There was a problem hiding this comment.
There is a lot of similar code between these functions, how could we D.R.Y. up our code?
|
|
||
|
|
||
| def to_json(self): | ||
| is_complete = True if self.completed_at else False; |
There was a problem hiding this comment.
Great choice to derive the value of is_complete from self.completed_at! We could also use the python bool function here instead of a ternary expression, what could that look like?
| # if request.args.get("sort") == "asc": | ||
| # goals = Goal.query.order_by(Goal.title.asc()) | ||
| # elif request.args.get("sort") == "desc": | ||
| # goals = Goal.query.order_by(Goal.title.desc()) | ||
| # else: |
There was a problem hiding this comment.
We want to remove commented code like this before opening PRs and rely on our repo's commit history if we need to look up past code.
| goals_response = [] | ||
| for goal in goals: | ||
| goals_response.append(goal.to_json()) |
There was a problem hiding this comment.
This would be a great place for a list comprehension:
goals_response = [goal.to_json() for goal in goals]Are there other places in the project that we fill a list where we could use list comprehensions?
| validated_task = [] | ||
| [validated_task.append(validate_task(task_id)) for task_id in request_body["task_ids"]] |
There was a problem hiding this comment.
This line might not be doing what you intend, line 70 fills validated_task because of the statement validated_task.append(validate_task(task_id)), but it also creates a new list that we don't keep. we could re-write this line something like:
validated_task = [validate_task(task_id) for task_id in request_body["task_ids"]]This line is a bit long, so we could create a variable to get the task_ids from the request_body:
task_ids = request_body["task_ids"]
validated_task = [validate_task(task_id) for task_id in task_ids]This feedback around creating a list that we don't end up using by wrapping the line in square brackets applies to line 75 below as well. How might we re-write that line?
| db.session.commit() | ||
|
|
||
|
|
||
| return make_response({"id" : goal.goal_id, "task_ids":request_body["task_ids"]}), 200 |
There was a problem hiding this comment.
There are some lines across the files that are a bit long for best practices, how could we split them up?
|
|
||
| db.session.commit() | ||
|
|
||
| PATH = "https://slack.com/api/chat.postMessage" |
There was a problem hiding this comment.
PATH is a local variable of the mark_completed function, so we'd typically see their name lowercased here. We use all caps for constant values, typically when declared where they're accessible across a file or project.
There was a problem hiding this comment.
Overall, really nice use of descriptive names and spacing to make the code easy to read!
No description provided.