kghalamb / AirStage

0 stars 0 forks source link

Code Review Comments (Alexander Specht) #13

Open Velevynn opened 11 months ago

Velevynn commented 11 months ago
  1. Use of SELECT * when querying rows repeated consistently in all .py files. Explicitly define return columns.
  2. Use of for-loops to iterate through Cursors for queries that are only intended to return a single object. There are specific SQL functions for returning single rows.
  3. Class object in book_venue is being passed in, but only to use its id in a query that then duplicates the attribute values of the object passed in.
  4. Identical book_venue functions, only with different argument values. Can modify external use of the functions to remove need for duplicate code.
  5. Unused ‘price’ variable within book_venue functions in book.py.
  6. Opening multiple database connections within the same function (delete_booking, edit_booking).
  7. Use of array indexing to access return value from query, use column names or other appropriate syntax (signup in user.py).
  8. Manually setting default values when creating performers/venues in signup. Supabase can define default values.
  9. Duplicate signup function declared? Router string is for signin, but function name is signup.
  10. Use of an If statement to set a single variable to false for a future single boolean If statement (book_venue in book.py). Can use the first if-statement as the conditional check instead of setting a value for the exact same check later.
  11. Implementation of deletion check in delete_bookings could result in an incorrect return if a booking is inserted after the booking is deleted but before the select statement is finished.
  12. Duplicate get_booking function in catalog.py. Router string mentions user_id, but function is named get_booking despite returning user info.
  13. (AS ABOVE) Duplicate get_venues function in catalog.py. Router string mentions performers and returns performer info.
  14. Storing the attributes from returned rows in new variables is unnecessary, since they can just be directly referenced when needed from the CursorResult or Sequence.
  15. No check or constraints to make sure the performer is not already booked during the given time in the book_venue functions.