Skip to content

fix: Use sum() instead of count() in HandCalculator.estimate_hand_value()#187

Closed
Apricot-S wants to merge 1 commit intoMahjongRepository:masterfrom
Apricot-S:fix/fix-bug-divider
Closed

fix: Use sum() instead of count() in HandCalculator.estimate_hand_value()#187
Apricot-S wants to merge 1 commit intoMahjongRepository:masterfrom
Apricot-S:fix/fix-bug-divider

Conversation

@Apricot-S
Copy link
Collaborator

@Apricot-S Apricot-S commented Feb 6, 2026

Collection has no count()

@Apricot-S Apricot-S requested a review from Nihisil February 6, 2026 02:14
@Apricot-S Apricot-S self-assigned this Feb 6, 2026
@Apricot-S Apricot-S added the bug label Feb 6, 2026
@Apricot-S Apricot-S changed the title fix: Use sum() instead of count() since Collection has no count() fix: Use sum() instead of count() in HandCalculator.estimate_hand_value() Feb 6, 2026
@Nihisil
Copy link
Contributor

Nihisil commented Feb 6, 2026

@Apricot-S actually I have uncommitted changes with hand optimizations and in my version it looks like that:

aka_dora_tiles = {FIVE_RED_MAN, FIVE_RED_PIN, FIVE_RED_SOU}
precomputed_aka_dora = sum(1 for t in tiles if t in aka_dora_tiles)

is it ok if we keep that version? I plan to make PR on next few hours, so this PR can be closed

@Apricot-S Apricot-S closed this Feb 6, 2026
@Apricot-S
Copy link
Collaborator Author

@Nihisil
Since x in xs returns bool and bool can be summed with sum, I think x in xs for ... is better than 1 for ... if x in xs. What do you think?

@Apricot-S Apricot-S deleted the fix/fix-bug-divider branch February 6, 2026 02:50
@Apricot-S
Copy link
Collaborator Author

1 for ... if may be more efficient because it doesn't have any extra elements.
Profiling would be needed to determine which is better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants