Maple: Rhyannon & Sabrina #46
Conversation
There was a problem hiding this comment.
Nicely done, Rhyannon and Sabrina! I was impressed by how consistent your return statements were. They were all uniform and used jsonify! Good habit to get into! Staying consistent in your return statements lets other users know how to use your API predictably.
A couple of things we could think about:
One thing we can do is make the return message more descriptive in these contexts:
try:
customer_id = int(customer_id)
except:
return jsonify(None), 400
Maybe let's change this from None to something that tells the user what they did wrong.
There are a few places I also suggested where we could create helper functions to DRY up our routes. Some of them are a little long with several different checks before the route even gets to its main job.
All those checks and error handling could be made into helper functions, which will shorten the length of the routes in general!
Other than that, your logic is good! Great job!
| id = db.Column(db.Integer, primary_key=True, autoincrement=True) |
| id = db.Column(db.Integer, primary_key=True, autoincrement=True) |
| id = db.Column(db.Integer, primary_key=True, autoincrement=True) |
|
|
||
|
|
||
| @customers_bp.route("", methods = ["GET"]) | ||
| def get_customers(): |
| if "name" not in request_body: | ||
| return jsonify(details="Request body must include name."), 400 | ||
| if "postal_code" not in request_body: | ||
| return jsonify(details="Request body must include postal_code."), 400 | ||
| if "phone" not in request_body: | ||
| return jsonify(details="Request body must include phone."), 400 |
There was a problem hiding this comment.
this would make a great helper function!
| if "title" not in request_body: | ||
| return jsonify(details="Request body must include title."), 400 | ||
|
|
||
| if "release_date" not in request_body: | ||
| return jsonify(details="Request body must include release_date."), 400 | ||
|
|
||
| if "total_inventory" not in request_body: | ||
| return jsonify(details="Request body must include total_inventory."), 400 |
There was a problem hiding this comment.
another good candidate for a helper function
|
|
||
| return jsonify(id=video.id), 200 | ||
|
|
||
| @videos_bp.route("/<video_id>/rentals", methods=["GET"]) |
| return jsonify(video.to_dict()), 200 | ||
|
|
||
|
|
||
| @videos_bp.route("/<video_id>", methods=["DELETE"]) |
| return jsonify(video.to_dict()), 200 | ||
|
|
||
|
|
||
| @videos_bp.route("/<video_id>", methods=["PUT"]) |
| return jsonify(new_video.to_dict()), 201 | ||
|
|
||
|
|
||
| @videos_bp.route("/<video_id>", methods=["GET"]) |
No description provided.