refactor: address code-quality audit (DRY, magic numbers, typed flags)#60
Merged
Conversation
Work the CODE_QUALITY_AUDIT.md backlog into the codebase. Behavior is unchanged; the full gate (compile, ktlint, detekt, unit tests) is green. Highlights: - util: centralize money/multiplier formatting (Money.kt), UI timestamp formatting (TimestampFormat.kt) and the share-sheet helper (ShareText.kt); wire them into the 9 game/wallet/history/profile/rounddetail/assistant ViewModels, removing the per-VM String.format/SimpleDateFormat copies. - ui: extract a shared game UI kit (GameHeader, StatCell, GameActionButton, RulesSheet, GameTokens) and collapse the five *RulesSheet files into thin data wrappers; the five game screens shrink ~15-23%. - ui: consolidate per-feature widgets onto one HairlineDivider/VerticalHairline, Modifier.cardSurface, SearchField and StackOutlineButton; promote the GameKey -> display label map to GameKey.displayName on the model; dedup the FilterChipRow branches. - data: DRY GameRepositoryImpl with newDraw(), an engine evaluate() helper, charge()/creditPayout() settlement guards, a gameRoundEntity() factory, safeMultiplier(), and ENGINE_UNAVAILABLE / ENGINE_CSV_DELIMITER constants. - ui: expose typed UiState flags (sessionProfitPositive, OutcomeTone) so the screens stop inferring profit/win-loss from formatted strings. - non-ASCII: remove all em-dashes and any unicode from comments/docstrings (unicode in visible UI strings is kept per design). - magic numbers: derive the KYC progress label, house-wallet reveal label and mines bounds message from their constants; name blackjack OPENING_CARDS / DOUBLE_FACTOR. - dead code: reuse the shared gridBackground() in Crash, drop a no-op Spacer, and document the lobby retry/use-cache no-ops. Deferred (noted for follow-up): per-game strategy split of GameRepository and RoundDetailViewModel, MonogramAvatar / IconButtonChip consolidation, and the relative-time ladder merge (kept separate to preserve distinct screen copy).
ProportionBar laid out both segments with Modifier.weight(wonFraction) and weight(1f - wonFraction). Compose rejects a zero/negative weight, so a profile with a 0% or 100% win rate (wonFraction 0f or 1f) crashed the screen with "invalid weight; must be greater than zero". Render each segment only when its share is non-zero. Pre-existing latent bug, surfaced while smoke-testing this branch on-device.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Behavior-preserving code-quality refactor that works the audit backlog into the
codebase: removes duplicated formatters and widgets, slims the five game
screens, tightens the game repository, and replaces string-based UI inference
with typed state. Net change is roughly -1300 lines.
Rationale
The five game screens and several ViewModels had drifted into near-copies. The
work consolidates the clear duplication where a shared abstraction genuinely
pays off, and intentionally stops short where it would not:
Money,TimestampFormat,ShareText) insteadof per-ViewModel
String.format/SimpleDateFormat/ share-intent copies.GameHeader,StatCell,GameActionButton,RulesSheet,GameTokens) so each screen keeps only its game-specific stage.The five
*RulesSheetfiles collapse to thin data wrappers.HairlineDivider/VerticalHairline,Modifier.cardSurface,SearchFieldand
StackOutlineButtonreplacing the per-feature redeclarations; theGameKey -> labelmap moves ontoGameKey.displayName.GameRepositoryImplkeeps its public interface but shares the repeated draw,engine-call, settlement-guard and entity-build logic via small private
helpers (
newDraw,evaluate,charge/creditPayout,gameRoundEntity,safeMultiplier) plus promoted constants.sessionProfitPositive,OutcomeTone) sothe screens tint profit / win-loss from semantics instead of inspecting the
formatted label text.
bounds message) is derived from the existing constants; blackjack gets named
OPENING_CARDS/DOUBLE_FACTOR.Trade-offs / deliberately deferred (kept out to avoid over-engineering and a
mixed PR):
GameRepository/RoundDetailViewModel:churning the 11-method public interface and all callers/tests is not worth the
gain right now.
MonogramAvatar/IconButtonChipconsolidation and the relative-time laddermerge (news vs notifications use intentionally different ladders, so merging
would change one screen's copy).
Verification
Full local gate, all green:
./gradlew :app:compileDebugKotlin./gradlew :app:compileDebugAndroidTestKotlin./gradlew :app:ktlintMainSourceSetCheck./gradlew :app:detekt./gradlew :app:testDebugUnitTest(entire unit suite, includingGameRepositoryImplTestand the game ViewModel tests)Test plan
stats strips, rules sheets and action buttons render as before.
Related issues
Checklist
feat/,fix/,chore/, etc.).