spruhaN / nutrition_pal

1 stars 1 forks source link

Code Review Comments (Sam Todd) #7

Open SamuelTodd opened 1 month ago

SamuelTodd commented 1 month ago
  1. Change SQL Alchemy return type used for POST /user endpoint – Since you are only inserting one customer at a time and returning only the newly created customer id, you can use “connection.execute(…).scalar_one()” to store the id itself in the result object.
  2. Standardize capitalization and SQL code style – The SQL code for some endpoints capitalizes the words for SQL commands while some others do not. For example, GET /workout/{workout_id}/muscle_groups and GET /workouts/muscle_groups/{type} do not capitalize the key words. For readability and consistency, I would recommend capitalizing all the key words for SQL commands across all endpoints.
  3. Combine SQL statements in POST /workout/{customer_id} – Instead of executing two separate SQL statements for retrieving the ID and then inserting the resulting value, you can do it all in one statement such as: “INSERT INTO customer_workouts (exercise_id, sets, reps, length, customer_id) SELECT id, :sets, :reps, :length, :customer_id FROM exercises WHERE name = :w_name”. Just make sure you include all necessary parameters for this statement.
  4. Remove unnecessary “result =” from POST /workout/{customer_id} and POST /meal/{customer_id} - If you are not returning anything, you do not need the “result =” before “connection.execute(…)” statements. You can remove them and structure the statements the same way they are in POST /goals/{customer_id}.
  5. Move all BaseModel objects to the top of their respective files – It may help to have all of the class objects that are created throughout your project in one location at the top of the file instead of having them throughout the file and between endpoints.
  6. Combine SQL statements in GET /daily_calories/{customer_id} – Instead of executing two separate SQL statements to do the calculation of daily calories left, you should combine them into one to reduce the number of separate calls made to the database. One way to do this would be to use “WITH” for each of the two statements followed by an additional “SELECT” to get the difference of the two values.
  7. Add descriptive comments to GET /meal/{customer_id}/day, GET /daily_calories/{customer_id}, POST /meal/{customer_id}, and GET /workouts/{customer_id}/day – Although there is additional information in the APISpec.md file, it would be helpful to have a quick description to reference when evaluating the accuracy of the code.
  8. Delete duplicate schema.sql from the root folder – As of now, the up-to-date schema.sql file is located within the website folder while an outdated schema.sql file exists in the main directory as well. The out-of-date file should be deleted for clarity.
  9. Create separate docs folder for all documents – Instead of having the documents such as the test results and ER diagram mixed in with actual application code, it would be best to move them to a separate folder.
  10. Create separate data folder for all default application data – Similarly to the previous recommendation, it is best to keep data and document files separate from the application code. Making a separate folder for all sample data needed to run and test the application would be best.
  11. Support exception handling – descriptive exceptions should be raised in the event that the endpoints fail to insert or retrieve the specified information. For example, if you fail to post a meal or workout to the database, a descriptive error should be raised that informs the user to try again later.
  12. Complete ingredient_id functionality in POST /meal/{customer_id} – The ingredient_id field is always being set to 1 in the current state of development. This is likely a placeholder for future feature development, since the schema.sql file specifies that ingredient_id will be generated in the ingredient table automatically. Since each meal can presumably have many ingredients, it might become necessary to insert into a meal_ingredient table during this endpoint as well.