Displaying Chip value contribution #6 #33

Merged
djwesty merged 10 commits from vutukuri15/6 into main 2025-02-25 11:00:20 -08:00
djwesty commented 2025-02-23 23:34:52 -08:00 (Migrated from github.com)

Work

This work addresses issue #6, to now show the chip denominations along with distributions.

  • Fixed the BuyInSelector to use a default of 25 rather than null
  • Added denomination logic to semi-arbitrarily pick a max denomination, then back-fill with the lower values
  • Some logic which attempts to re-denominate as needed
  • Distribution logic, which distributes one chip at a time from highest to lowest value, until there are no more chips of that color to distribute, or not enough remaining value to allow the particular chip
  • Fix existing tests to work with component

Limitations

This feature works, but there are some room for improvements. These will be documented in an issue for next sprint, but to summarize, a more complex and optimized algorithm probably could be made

  • The total value is usually correct, but sometimes falls short of the buy-in. If this case cannot be optimized away, some UI feedback to note it would be good (improve distribution)
  • Optimizations to put better emphasis on thoughtfully using more lower value chips (improve denominations)
  • Explore the possibly to abstract or decouple this otherwise tightly coupled feature/file
### Work This work addresses issue #6, to now show the chip denominations along with distributions. - Fixed the `BuyInSelector` to use a default of 25 rather than null - Added denomination logic to semi-arbitrarily pick a max denomination, then back-fill with the lower values - Some logic which attempts to re-denominate as needed - Distribution logic, which distributes one chip at a time from highest to lowest value, until there are no more chips of that color to distribute, or not enough remaining value to allow the particular chip - Fix existing tests to work with component ### Limitations This feature works, but there are some room for improvements. These will be documented in an issue for next sprint, but to summarize, a more complex and optimized algorithm probably could be made - The total value is usually correct, but sometimes falls short of the buy-in. If this case cannot be optimized away, some UI feedback to note it would be good (improve distribution) - Optimizations to put better emphasis on thoughtfully using more lower value chips (improve denominations) - Explore the possibly to abstract or decouple this otherwise tightly coupled feature/file
MantashaNoyela (Migrated from github.com) reviewed 2025-02-23 23:34:52 -08:00
Vutukuri15 commented 2025-02-25 09:12:16 -08:00 (Migrated from github.com)

Great job on implementing this feature! The way denominations are chosen and chips are distributed is well thought out, and the re-denomination logic is a useful addition.
The identified limitations make sense, and optimizing the distribution for better balance while ensuring the total value matches the buy-in would be great next steps.
Overall, great work—looking forward to seeing the refinements in the next sprint!

Great job on implementing this feature! The way denominations are chosen and chips are distributed is well thought out, and the re-denomination logic is a useful addition. The identified limitations make sense, and optimizing the distribution for better balance while ensuring the total value matches the buy-in would be great next steps. Overall, great work—looking forward to seeing the refinements in the next sprint!
Vutukuri15 (Migrated from github.com) approved these changes 2025-02-25 09:12:47 -08:00
Sign in to join this conversation.
No description provided.