NickPerlich / Bettermo

1 stars 1 forks source link

Code Review Comments Sebastian Thau #3

Open Sebi2k opened 11 months ago

Sebi2k commented 11 months ago
  1. Error Handling - Consider implementing error handling. For example, what should happen if a user tries to add a duplicate group or if the provided group_id or user_id does not exist?
  2. Documentation - It would be nice if there were some explanations for the more complex SQL statements. Also, just general notes for what does what.
  3. Decimal Types for Currency - Since you are dealing with financial data (price in cents), consider using the Decimal type instead of float to avoid precision issues.
  4. HTTP Exception - Consider adding error handling to deal with potential issues with database transactions. If an exception occurs, it's helpful to return an appropriate HTTP response code and potentially include more details in the response
  5. Consistent Naming - try to maintain consistency in naming conventions. For example, in your post_group_purchase function, you use purchase_id and transaction_ids. While this is clear, it's a good idea to stick to a consistent naming convention throughout your code.
  6. Validation - Maybe add validation for input parameters, especially in the case of user input (e.g., validate that group_id and user_id are positive integers)
  7. SQLalchemy ORM - This is preference, but maybe consider using SQLalchemy ORM instead of raw SQL queries. It can provide a more Pythonic and high-level interface to interact with your database.
  8. API Path Parameters - In your path parameters, you guys could use more descriptive names than group_id and user_id, for example, group_id could be group_id_path and user_id could be user_id_path.
  9. Return Values - Consider returning success/failure responses. For example, in the post_group_purchase function, you might return something indicating the success or failure of the operation.
  10. Unused Imports - Remove commented-out import statements, such as #from src.api import auth. It's good to keep the codebase clean.
  11. Transaction Rollback - Consider implementing a rollback mechanism in case any part of the transaction fails
  12. Logging - Maybe add logging to help with debugging and monitoring.