Skip to content

refactor: extract dispatch dict and _handle_reaction in _handle_message() (closes #451)#459

Merged
mholzi merged 1 commit intomainfrom
fix/issue-451
Mar 31, 2026
Merged

refactor: extract dispatch dict and _handle_reaction in _handle_message() (closes #451)#459
mholzi merged 1 commit intomainfrom
fix/issue-451

Conversation

@mholzi
Copy link
Copy Markdown
Owner

@mholzi mholzi commented Mar 31, 2026

Fixes #451 — refactors the 252-line _handle_message() into a dispatch dict pattern and extracts _handle_reaction(), removing PLR0912/PLR0915 noqa suppressions.

…ge() (closes #451)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the WebSocket message handling in custom_components/beatify/server/websocket.py by replacing a large conditional block with a dispatch dictionary and dedicated handler methods. This change improves code organization and reduces method complexity. The review feedback suggests utilizing the existing get_player_by_ws helper method in the admin handler to replace a manual iteration, enhancing code efficiency and consistency.

Comment on lines +338 to +342
sender = None
for player in list(game_state.players.values()):
if player.ws == ws:
sender = player
break
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The manual iteration to find the player session by WebSocket can be replaced with the more efficient game_state.get_player_by_ws(ws) method. This improves readability and leverages the existing API already used in other handlers like _handle_reaction.

Suggested change
sender = None
for player in list(game_state.players.values()):
if player.ws == ws:
sender = player
break
sender = game_state.get_player_by_ws(ws)

Copy link
Copy Markdown
Owner Author

@mholzi mholzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review: LGTM — clean refactor.

Dispatch dict: all 11 original if/elif branches correctly mapped; unknown types still log a warning.

Extracted methods: _handle_reaction(), _handle_join(), _handle_admin(), _handle_get_state() all preserve original logic exactly. Signatures are consistent with existing handler patterns (ws, data, game_state).

Signature normalization: _handle_leave and _handle_get_steal_targets correctly gained the unused data param for uniform dispatch — confirmed no other callers exist.

noqa cleanup: PLR0912/PLR0915 suppressions removed as expected.

No regressions, edge cases, or correctness issues found.

@mholzi mholzi merged commit b246a08 into main Mar 31, 2026
5 checks passed
@mholzi mholzi deleted the fix/issue-451 branch March 31, 2026 09:18
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

Successfully merging this pull request may close these issues.

[Code Review: code-quality] _handle_message() is 252 lines with noqa suppressions — needs dispatch refactor

1 participant