Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Game table changes #28

Closed
avadhanij opened this issue Jul 17, 2021 · 9 comments
Closed

Game table changes #28

avadhanij opened this issue Jul 17, 2021 · 9 comments

Comments

@avadhanij
Copy link

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
Copy link
Owner

mpope9 commented Jul 18, 2021

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
Copy link
Owner

mpope9 commented Jul 19, 2021

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
Copy link
Author

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
Copy link
Owner

mpope9 commented Jul 19, 2021

@avadhanij Wow I completely missed that there was no requester for team_game_log. I created this issue to track that #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
Copy link
Author

avadhanij commented Jul 20, 2021

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
Copy link
Owner

mpope9 commented Jul 20, 2021

@avadhanij yeah, I like that better.

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

@avadhanij
Copy link
Author

avadhanij commented Jul 20, 2021

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
Copy link
Owner

mpope9 commented Jul 20, 2021

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

@mpope9
Copy link
Owner

mpope9 commented Jul 31, 2021

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

@mpope9 mpope9 closed this as completed Jul 31, 2021
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

No branches or pull requests

2 participants