In attack_monster in hero.pyI suggest including conditional rendering for the response in checking if updating the monster's health is successful. Can the monster have negative health? Consider edge cases like this where subtracting monster's health and hero's power might not yield a successful "OK" return. Per the API spec, the response should be "success": boolean; thus, include functionality + checks for success being false.
In recruit_hero(), consider using Try/Except block to handle possible errors, such as if the hero is not found or already in a guild, rather than simply checking the row count and outputting a message based on that.
create_guild() in guild.py: Include error handling to check if guild is actually properly created. Considering potential errors in types passed (ex, max_capacity should be an int per Class design, but what if it's passed as an unintended type?). In other words, consider cases where "success" is not True, and throw proper errors.
infind_heroes include a try/except block to ensure the dungeon id passed in is valid (ie, it exists/is a valid integer per given type)
in check_health.py include a try/except block to check for valid input type and existing hero_id
In create_monster, eliminate hard coding return to True. Include error handling to actually see if successful creation or not.
In send_party write error-handling logic to see what specific error is actually occurring, rather than a message of the form ".. Or .."
Likewise, in send_request refrain from having one general error message. Instead, write error handling logic to see which of the three (or more) possible errors actually are. This will be super helpful for debugging.
In raise_level have a more useful response. Consider returning the new or added/subtracted xp and level along with success.
Class 2 - Code Design
I noticed that you use result as the target variable for the majority of API calls. This works, but for readability and best practices, instead set the output of the call to a more descriptive variable name. Ex: new_level = ..
In the code below (in monster.py find_heroes()), it is best practice to use '.' (dot) notation, rather than indexing.
Include comments and docstrings (with full request, response headers) in each function. It is important to consider readability as well as functionality! Make this consistent across all endpoints.
in the code below, eliminate the unnecessary list [] of dictionaries. After the comma on the bottom line, you can simply have: , {"hero_id": hero_id}
result = connection.execute(sqlalchemy.text
("""
UPDATE hero
SET level = level + 1, xp = xp - 100
WHERE id = :hero_id AND xp >= 100
"""), [{"hero_id": hero_id}])
12 Code Review Comments by Ryan Agdassi
Class 1 - Error Handling
In
attack_monsterinhero.pyI suggest including conditional rendering for the response in checking if updating the monster's health is successful. Can the monster have negative health? Consider edge cases like this where subtracting monster's health and hero's power might not yield a successful "OK" return. Per the API spec, the response should be "success": boolean; thus, include functionality + checks for success being false.In
recruit_hero(), consider using Try/Except block to handle possible errors, such as if the hero is not found or already in a guild, rather than simply checking the row count and outputting a message based on that.create_guild()in guild.py: Include error handling to check if guild is actually properly created. Considering potential errors in types passed (ex, max_capacity should be an int per Class design, but what if it's passed as an unintended type?). In other words, consider cases where "success" is not True, and throw proper errors.in
find_heroesinclude a try/except block to ensure the dungeon id passed in is valid (ie, it exists/is a valid integer per given type)in
check_health.pyinclude a try/except block to check for valid input type and existing hero_idIn
create_monster, eliminate hard coding return to True. Include error handling to actually see if successful creation or not.In
send_partywrite error-handling logic to see what specific error is actually occurring, rather than a message of the form ".. Or .."Likewise, in
send_requestrefrain from having one general error message. Instead, write error handling logic to see which of the three (or more) possible errors actually are. This will be super helpful for debugging.In
raise_levelhave a more useful response. Consider returning the new or added/subtracted xp and level along with success.Class 2 - Code Design
I noticed that you use result as the target variable for the majority of API calls. This works, but for readability and best practices, instead set the output of the call to a more descriptive variable name. Ex:
new_level= ..In the code below (in
monster.pyfind_heroes()), it is best practice to use '.' (dot) notation, rather than indexing.Include comments and docstrings (with full request, response headers) in each function. It is important to consider readability as well as functionality! Make this consistent across all endpoints.
in the code below, eliminate the unnecessary list [] of dictionaries. After the comma on the bottom line, you can simply have: , {"hero_id": hero_id}