Skip to content

refactor: remove else clauses using Object Calisthenics #2#2817

Open
odanilosalve wants to merge 3 commits into
soxoj:mainfrom
odanilosalve:refactor/clean-imports-and-remove-else
Open

refactor: remove else clauses using Object Calisthenics #2#2817
odanilosalve wants to merge 3 commits into
soxoj:mainfrom
odanilosalve:refactor/clean-imports-and-remove-else

Conversation

@odanilosalve

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #2816 (clean imports + next(iter())). Applies Object Calisthenics rule #2 (no else keyword) to maigret/checking.py, with the readability fixes the reviewer asked for.

Patterns applied

  • Dict mapping: HTTP method selection collapses to a single getattr (no more per-call identity dict rebuilt on every request):
    method = method.lower()
    request_method = getattr(
        session, method if method in ('get', 'post', 'head') else 'get'
    )
  • Guard clauses: make_site_result returns early for disabled sites, mismatched identifier types, and disallowed usernames — no more if/elif/else ladder.
  • Early returns: SSL exception handling, payload form/json branching, request_method defaulting, and the allow_redirects ternary.
  • Inline assignment: keyword_match_status is set in the same if arm that decides it (no separate line with a long ternary).
  • Dict dispatch: process_site_result uses a _checkers dict instead of an if/elif/elif/elif/else chain. Each helper uses early returns — none of the long one-line ternaries that read worse than the original if/else.

Readability fixes from review

  • _check_message / _check_status_code / _check_response_url use guard-then-return, not one-line ternaries.
  • keyword_match_status is set inside the if/else arms instead of via a long ternary.
  • is_form ternary is short and reads fine; left as-is.

Net change

-12 net lines in checking.py, 314 tests pass, 3 skipped (network-dependent).

Depends on #2816 for the import reorganization + next(iter()) cleanup.

Two safe, mechanical changes:

- activation.py: replace `list(domain.values())[0]` with
  `next(iter(domain.values()))` in import_aiohttp_cookies to avoid
  materializing the entire iterable just to grab the first dict.
- checking.py: reorganize imports to PEP 8 order (stdlib, third-party,
  local). `from unittest.mock import Mock` and the stdlib `quote`
  import move into the stdlib section; `python_socks` and
  `socid_extractor` move into third-party; `from .error_detection import
  detect_error_page` and the `from .` local imports move into the local
  section.

No behavioral change. Same diff shape as 2484509 -> HEAD for these two
files minus the else-removal work.
Applied Object Calisthenics rule soxoj#2 (no `else` keyword) across the core
detection engine with the patterns called out in the review:

- Dict mapping: HTTP method selection collapses to a single
  `getattr(session, method if method in (...) else 'get')` — no more
  per-call identity dict.
- Guard clauses: `make_site_result` now returns early for disabled sites,
  mismatched identifier types, and disallowed usernames. No nested
  `if/elif/else` ladder.
- Early returns: SSL exception handling, payload form/json branching,
  request_method defaulting, and the `allow_redirects` ternary.
- Inline assignment: `keyword_match_status` is set in the same `if` arm
  that decides it (no separate line with a long ternary).
- Dict mapping with helper functions: `process_site_result` dispatches
  via a small `_checkers` dict instead of an `if/elif/elif/elif/else`
  chain. Each helper uses early returns for clarity.

The helper bodies (`_check_message`, `_check_status_code`,
`_check_response_url`) deliberately avoid one-line ternaries — they
read better as guard-then-return.

Net: -34 lines, 300 tests still passing.
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.

2 participants