jluudev / arthurslastcrusade

https://arthurslastcrusade.onrender.com
0 stars 0 forks source link

Code Review (Justin Yuen #34

Open justdanyuen opened 4 months ago

justdanyuen commented 4 months ago
  1. In hero.py, accept_request, when an error is reached during table entry determine the specific issue with the entry rather than a default print statement that covers multiple possible criteria
  2. In API spec, when a hero dies they are removed from the guild. However, in the code, it is unclear where the reference of guild changes / is removed
  3. Error Handling - In dungeon.py, implement a check to limit the level that a monster can be so that unreasonably high levels (and negative levels) are rejected. Insert an if statement check before inserting it into the table.
  4. Update API spec to properly correlate variables to the created monster expected, currently just calls for "type".
  5. Error Handling - In hero.py, implement a check for updating health values in attack-monster. When a monster is attacked, check if the value is negative and update other columns (ie. the alive column) when conditions are met.
  6. Add more comments throughout methods with frequent calls (dungeon.py, hero.py, monster.py) to help with debugging. Currently, there are no status updates of functions.
  7. Ensure that in monster.py if an action (ie. die) fails, transactions to other tables also fail accordingly and there is consistency in updating tables when errors occur.
  8. Consider using a ledger to monitor health over time rather than updating the value itself to have a record of changes throughout the hero/monster's life.
  9. In monster.py, in the method find_heroes there is a select function to get the details of the heroes. Instead of using indexing to retrieve the data, it's better to practice to query the attributes by column name to prevent potential bugs from incorrect/invalid indexing (ie. row.level instead of row[2])
  10. For many of the methods (ie. /find_heroes/{dungeon_id}) there is a lack of error handling for if the code is unable to connect to the database. Nest the existing code with a try-except block to prevent unwanted errors.
  11. In the file monster.py, there is no mechanism for logging which is essential for debugging and monitoring API usage.
  12. In world.py, ensure that data coming from clients through API requests is validated beyond just using pydantic models. This can include checking for valid IDs, permissible values, etc.