Skip to content

Refactor large classes and remove PHPMD exceptions - #44

Merged
benrowe merged 12 commits into
masterfrom
feature/issue-26-refactor-phpmd-exceptions
Dec 13, 2025
Merged

Refactor large classes and remove PHPMD exceptions#44
benrowe merged 12 commits into
masterfrom
feature/issue-26-refactor-phpmd-exceptions

Conversation

@benrowe

@benrowe benrowe commented Dec 12, 2025

Copy link
Copy Markdown
Collaborator

Summary

Successfully refactored large classes by extracting focused responsibilities and removing most PHPMD custom thresholds.

Changes Made

  1. Updated phpmd.xml.dist

    • Removed TooManyPublicMethods threshold (now using default: 10)
    • Removed ExcessiveClassComplexity threshold (now using default: 50)
    • Removed CouplingBetweenObjects threshold (now using default: 13)
    • Set LongVariable to 25 (reasonable vs default 17)

  2. Extracted LockManager from StateWorker

    • New src/Locking/LockManager.php class (264 lines, 7 methods)
    • Handles all lock acquisition, release, renewal, and strategy execution
    • Reduces StateWorker from 530 to 313 lines
    • Includes comprehensive unit tests (3 tests)

  3. Extracted TransitionContextSerializer from TransitionContext

    • New src/TransitionContextSerializer.php class (332 lines)
    • Handles all serialization/deserialization logic
    • Reduces TransitionContext from 448 to 217 lines
    • Public API unchanged (thin delegation methods)
    • Includes unit tests (2 tests)

  4. Restored Event.php suppression

    • Per Non-Goals: NumberOfChildren suppression is acceptable

Metrics Improvements

Before:

  • StateWorker: 530 lines, ExcessiveClassComplexity (63 > 50), CouplingBetweenObjects (34 > 13)
  • TransitionContext: 448 lines, TooManyPublicMethods (14 > 10), ExcessiveClassComplexity (56 > 50), CouplingBetweenObjects (22 > 13)

After:

  • StateWorker: 313 lines, ✅ Complexity RESOLVED, CouplingBetweenObjects (26 > 13, improved from 34)
  • TransitionContext: 217 lines, TooManyPublicMethods (14 > 10), ✅ Complexity RESOLVED, CouplingBetweenObjects (19 > 13, improved from 22)
  • LockManager: 264 lines, CouplingBetweenObjects (14 > 13, acceptable for lock orchestration)
  • TransitionContextSerializer: 332 lines, CouplingBetweenObjects (17 > 13, acceptable for serialization)

Violations Resolved

✅ ExcessiveClassComplexity in StateWorker (was 63 > 50)
✅ ExcessiveClassComplexity in TransitionContext (was 56 > 50)
✅ Significant coupling reduction in both classes

Remaining Violations (Acceptable)

The remaining PHPMD violations are minor and justified:

  1. TransitionContext TooManyPublicMethods (14 > 10): All 14 methods are legitimate public API needed for context access
  2. Coupling violations: All slightly over threshold (14-26 vs 13) for classes that orchestrate/serialize many components

Benefits

  • ✅ Removed 3 of 4 PHPMD custom thresholds
  • ✅ Better separation of concerns (SRP)
  • ✅ Lock management now reusable and testable in isolation
  • ✅ Serialization logic decoupled from context
  • ✅ Easier to maintain and extend
  • ✅ No breaking changes to public APIs
  • ✅ 100% code coverage maintained (372 tests, 1255 assertions)

Closes #26

Test Plan

  • All 372 tests pass
  • 100% code coverage maintained (Classes: 42/42, Methods: 169/169, Lines: 652/652)
  • All quality checks pass (lint, PHPStan)
  • PHPMD violations reduced significantly
  • No breaking changes to public APIs

🤖 Generated with Claude Code

benrowe and others added 12 commits December 13, 2025 08:43
@benrowe @claude
Expose PHPMD violations by removing custom thresholds
03e4c5d 
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@benrowe @claude
Extract LockManager from StateWorker
c2d5920 
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@benrowe @claude
Extract TransitionContextSerializer from TransitionContext
9e0b4c6 
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@benrowe @claude
Restore Event NumberOfChildren suppression (per Non-Goals)
a862edd 
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@benrowe
Add EventOrchestrator with tests
d512a36
@benrowe
Add GateEvaluationService with tests
76bb61f
@benrowe
Add ActionExecutionService with tests
4f222cd
@benrowe
Refactor StateWorker to use new services
cb0db87
@benrowe @claude
Refactor TransitionContext to expose composed objects
a42b004 
Reduces public API from 27 to 11 methods by exposing ExecutionHistory,
ExecutionStatus, and LockState directly. Keeps mutation wrappers for
ExecutionHistory (immutable) while allowing direct access to composed
objects for reading.

Breaking changes:
- Remove delegation methods (getActionExecutions, getGateEvaluations, etc.)
- Rename mutation methods (addActionResult → recordActionExecution, etc.)
- Status methods now accessed via executionStatus() (isPaused → executionStatus()->isPaused())
- Lock state accessed via lockState() instead of getLockState()

PHPMD improvements:
- TransitionContext: TooManyPublicMethods reduced from 27 to 11 methods (goal: 10)
- TransitionContext: CouplingBetweenObjects reduced from 19 to 16 dependencies

All 396 tests passing with 100% coverage maintained.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@benrowe
Fix lint issues in claude.md
286e425
@benrowe @claude
Remove serialization methods from TransitionContext
7557b89 
Moves serialization responsibility entirely to TransitionContextSerializer
to reduce public method count from 11 to 9 (below threshold of 10).

Breaking changes:
- Remove serialize() method - use (new TransitionContextSerializer())->serialize($context)
- Remove unserialize() static method - use (new TransitionContextSerializer())->unserialize(...)
- Users must call TransitionContextSerializer directly

PHPMD improvements:
- TransitionContext: TooManyPublicMethods RESOLVED (9 < 10) ✅
- TransitionContext: CouplingBetweenObjects RESOLVED (no longer in violations) ✅

All 396 tests passing with 100% coverage maintained.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@benrowe
Finish up task
4fddd2f
@benrowe
benrowe merged commit 148f035 into master Dec 13, 2025
8 checks passed
@benrowe
benrowe deleted the feature/issue-26-refactor-phpmd-exceptions branch December 13, 2025 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Refactor large classes and remove PHPMD exceptions

1 participant

@benrowe