hlathery / Virtual-Ecosystem

0 stars 0 forks source link

Schema/Api Design Comments (Timothy Matthies) #23

Open TLMatthies opened 4 days ago

TLMatthies commented 4 days ago
  1. I'd like to keep the timestamps in the tables, and also start aiming for legerization. Makes it so much easier to debug and test your database.
  2. In your code you make reference to the fact that there are three specific biomes, but I do not see those being inserted in the schema, so I'm not sure how those are supposed to be used and kept track of.
  3. Got an error when trying out GET /info/current_time. GET methods should not have a body passed with them.
  4. Odd return objects on GET /village/ and GET /village/catalog. I'd guess it works, but I'd like to see buildings and num/cost of paired up and easily readable.
  5. A lot of API calls are redundantly named. I.e.: PUT /eco/spawn_prey or GET /assignments/get_job_list. The spawn/put and get are already known because of what HTTP method they are, there's not much of a need to include it in the name. I'd recommend renaming them to something like PUT /eco/prey and GET/assignments/jobs so that it's easier to tell what resource they operate on, and makes it feel less cluttered when view all of the API names. I also learned that this is a standard practice in industry from my CSC 307 class.
  6. Maybe change POST /village/kill_villager to DELETE /village/villager so it's clear from the HTTP method what's supposed to happen.
  7. I'm confused about why PUT /eco/grow_plants is a PUT, while all the other similar (i.e. spawn_prey) methods are POSTs. Is there a design choice to this?
  8. Change the description of PUT /assignments/assign_villager. It makes no sense and describes the method incorrectly.
  9. What is the point of adding "job_title" in the PUT /assignments/assign_villager? I was able to change it to whatever I wanted and it still only cared about the job_id.
  10. Getting a 500 error on POST /eco/biomes , while using it as I believe is intended. Passed: { "Ocean":4, "Forest":6}
  11. Got a 500 error on GET /eco/plants
  12. Got a 500 error on GET /eco/prey