Skip to content

Code Review Comments Baylor Pond #23

@BPond4

Description

@BPond4

Inconsistent Sqlalchemy queries

Your sql queries within drafts.py are inconsistent as far as formatting. It is probably best to pick a format and stick with it within the sqlalchemy.text() call.

Query Builder vs Sqlalchemy.text

Across files you are making sql queries in different ways. In drafts.py you guys are using sqlalchemy.text() but in players.py you guys are using sqlalchemy's query builder. It obviously works but it would be an improvement to keep things consistent across the codebase.

Assert false instead of exception

On line 85 in players.py you use a assert false instead of an exception that could provide more information to the user or debugger when that line is reached.

Mappings on a query with one return row

In this code in drafts.py, you use .mappings() on the query you made however the query will only ever return one row because you are specifying a draft id. Because of this mappings() is not helpful
draft = connection.execute(sqlalchemy.text(""" SELECT draft_size, draft_status FROM drafts WHERE draft_id = :id AND draft_status = 'pending' """), {"id": draft_id}).mappings().fetchone()

Larger comments for each function

A comment at the beginning of each function or endpoint with a more in depth explanation can be very helpful for others looking to quickly understand the code.

Endpoint that could be more readable

This endpoint has a very long chunk of variable assignment after the query that makes the endpoint less readable. This section could be sourced to a helper function or something.

def get_player_statistics(player_id: str):

Try/except for more error details

If you wrap your query and database calls in try/except statements, you can more easily handle errors and return more details about them.

Repurposing commented out code

The commented out code at the bottom of some files seems very useful and could be put in a different file for testing

Unnecesasry files

Some files in the codebase are not relevant to the project and just add clutter

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