Skip to content

Code Review Comments Muzart Tuman #18

@ZealSV

Description

@ZealSV

Great stuff here, I'll be very nit picky so bear with me.

1. Comments like (# print id for testing), (# return id), or (# query database) in drafts could be removed as the functions seem pretty tell tale.
------------------------------------------------------------------------------------------------------------
2. You could wrap your SQL queries in try except blocks to get further error details.

try: ...

except sqlalchemy.exc.SQLAlchemyError as e:
raise HTTPException(status_code=500, detail=str(e))
------------------------------------------------------------------------------------------------------------
3. Use Pydantic and declare all these at the top of your code, which you've already done with some but more can be included like: (helps with autogeneration of documentation)

class DraftRoomResponse(BaseModel):
draft_id: int
draft_name: str
draft_type: str
draft_size: int
roster_size: int
draft_length: int
flex_spots: int

class JoinDraftResponse(BaseModel):
team_id: int

class StartDraftResponse(BaseModel):
success: bool
message: str

class EndDraftResponse(BaseModel):
success: bool
------------------------------------------------------------------------------------------------------------
4. More descriptive comments at the top of functions would assist to better documentation in your code.
------------------------------------------------------------------------------------------------------------
5. Define variables outside of SQL statements for cleaner code.
------------------------------------------------------------------------------------------------------------
6. Validation constraints can be added to some models like the RosterPosition model for better error handling.
------------------------------------------------------------------------------------------------------------
7. You can use transactions in the drafts.py file, with your post_reqs var you can insert the appended item within the list as well as the for statement to make sure it completes or fails without some half fail half successes.
------------------------------------------------------------------------------------------------------------
8. You can add logging to track errors to see whats going wrong more.
------------------------------------------------------------------------------------------------------------
9. Delete Central Coast Cauldron files which can be removed.
------------------------------------------------------------------------------------------------------------
10. You could possibly implement asynchronous functions as well as asynchronous SQL queries to improve overall performance and have that feature (improves scalability).
------------------------------------------------------------------------------------------------------------
11. You could implement a limit to how many requests a user is calling for a certain amount of calls per a certain amount of time.
------------------------------------------------------------------------------------------------------------
12. Though the random.shuffle will definitely work in your case, you could implement a near redundant "if previous = current, shuffle again" logic to guarantee the first shuffle will not match the second.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions