search

teamnzs / ios-trivia-game

iOS Trivia Game
0 stars 0 forks source link

Issue/19 game options #33

Closed wassupnari closed 7 years ago

wassupnari commented 7 years ago

In this PR, I'm getting Trivia Categories from JService, and populate them in pickerview.

issue19

Let me know if you guys notice anything!

saviotsui commented 7 years ago

Perhaps I should have mentioned this in your previous PR for friends view controller. If we specify private game (not public) is the logic to check that we have added the total number of people implemented?

I think right now it might be possible to also over select the max number of players

saviotsui commented 7 years ago

I was curious, are you planning to make the firebase db integration in a separate pr once you've gone through all the game setup view controllers? I don't see any code to update the DB yet

wassupnari commented 7 years ago

Hi Savio,

For your question about max number of users, yes! I'm sending max number of users to SelectFriendsViewController, but I don't has guard to prevent selecting more than max number. I'll add that logic.

Also, for the Firebase api call - I was planning on updating DB at the end of creating game process, and I just realized that this page is the final. Should I include that in this PR then?

saviotsui commented 7 years ago

Okay submit separate Firebase API Call PR!

wassupnari commented 7 years ago

Thanks! Will do that tonight!

I made other changes that you guys addressed. :)

zhiachong commented 7 years ago

Added a few more things I noticed... Overall looks good.

wassupnari commented 7 years ago

Hi @zhiachong I just made changes for removing magic numbers. For abstraction though, we need to subclass UIPickerViewDelegate and inject the dependencies for tag information, but I honestly don't know how to do that in Swift, and it think that's overkill for this project. Also, current implementation is very common pattern, and I don't think it's creating any boilerplate code.

Let me know what you think!