search

mpope9 / nba-sql

:basketball: An application to build an NBA database backed by MySQL, Postgres, or SQLite
Apache License 2.0
176 stars 20 forks source link

Game table changes #28

Closed avadhanij closed 3 years ago

avadhanij commented 3 years ago

I feel it's missing one important column - winner, or rather winner_team_id. Putting that together right now would require a few more unnecessary joins.

I was also thinking the two columns team_id_home_id, and team_id_away_id could be renamed to home_team_id and away_team_id.

What do you think?

mpope9 commented 3 years ago

While it does exist in team_game_log, I can see how adding it would be helpful. Maybe team_id_winner and team_id_loser? Game is a touch Frankenstein, is there a 'clean' way to do this?

For the rename, maybe removing the last _id would make sense. Hungarian notation was a goal, but I wasn't the best following it.

mpope9 commented 3 years ago

I'm not super sure about this anymore, due to ties. I'm not a huge fan of nullable foreign keys. In my eyes, a join to check the wl field isn't the worst thing.

avadhanij commented 3 years ago

So, team_game_log endpoint is currently not implemented. Technically, if you are fine with that instead, then I can add that endpoint and table, and yes, you could just infer from here. I have to think about this one a bit more as well.

Having said that, adding winner_team_id and loser_team_id is fine right? Why would they be null? Every game has a winner and loser.

mpope9 commented 3 years ago

@avadhanij Wow I completely missed that there was no requester for team_game_log. I created this issue to track that https://github.com/mpope9/nba-sql/issues/29.

Yeah, you're right there would be no null. I'm good with adding those to the game table, then. winner_team_id and loser_team_id is good as well.

avadhanij commented 3 years ago

Cool. This is an interesting, and good table. I don't think any of the endpoints give you visibility into things like home/away team win loss stats. I will add these two columns.

As for the style, I see what you were attempting, <type>_<name>, hence team_id_home_id. I think what you proposed is decent. So it will be -

  1. team_id_home
  2. team_id_away
  3. team_id_winner
  4. team_id_loser
mpope9 commented 3 years ago

@avadhanij yeah, I like that better.

Where are you getting it from? Is it from team_game_log?

avadhanij commented 3 years ago

That's one way. Another way would be to just expand on the player_game_log logic you already put in, but it's a bit wonky.

So, when you are constructing an entry for a game, we could make use of the WL field in the response. A W would mean I just pass the current player's team id for winner, but loser is empty. We can then figure out the loser inside the GameBuilder. Vice-versa if it's L.

The benefit with this approach is that we don't need to hit two end points to construct one table. It will get trickier if a user opts to skip building the team_game_log table. We will be forced to still hit it, but then skip only the populating portion.

What do you think?

mpope9 commented 3 years ago

That works for me. Creating two maps and fill them with game_id -> team_id incrementally doesn't sound too gnarly.

mpope9 commented 3 years ago

I think we can close this one, after the addition of the w/l ids to the game table.