Skip to content

Add new boolean fields to Game model#497

Open
jgdhs27 wants to merge 3 commits inton-rook:mainfrom
jgdhs27:consts
Open

Add new boolean fields to Game model#497
jgdhs27 wants to merge 3 commits inton-rook:mainfrom
jgdhs27:consts

Conversation

@jgdhs27
Copy link
Collaborator

@jgdhs27 jgdhs27 commented Dec 30, 2023

Adds 3 new fields to each game:

  1. has_routes
  2. has_multiple_shots
  3. has_subshots

Also simplify some logic using the new fields.

@jgdhs27 jgdhs27 marked this pull request as ready for review December 30, 2023 23:46
@n-rook
Copy link
Owner

n-rook commented Jan 1, 2024

This is a good idea, but it's a bit scary. The reason it's a bit scary is that it denormalizes the database: these fields can be wrong according to other tables in the database. In theory, we could instead create a has_routes() function, for example, that goes looking in the Routes table to see if the game has routes or not. This would be safer but slower. (In theory, there exist constructs in databases such as materialized views to split the difference here, but I looked into this and it seems like it would be a pain in the ass without much native Django support, so I don't think it's worth doing.)

I think it is fine to do things this way, on one condition: We need a test that validates that for each game, these fields are correct. In other words, it should create the constant table and then double check that, for example, there are routes if has_routes is set, and no routes if it isn't set. Since these are all constants anyway, a test suffices to check their correctness.

As a side note, I would consider computing these fields in setup_constant_table rather than treating them as fixed constants. But you don't have to do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants