michaelschiemer/docs/claude/framework-refactoring-recommendations.md

Framework Refactoring Recommendations

Comprehensive analysis and refactoring suggestions for the Custom PHP Framework.

Executive Summary

Framework Analysis Results:

• Total PHP Files: 2,469 files across 90+ modules
• Readonly Classes: 1,550 (excellent immutability compliance)
• Exception Files: 82 custom exceptions
• Initializers: 65 initialization classes
• Service Interfaces: 24+ service contracts
• Value Object Directories: 49 dedicated VO namespaces

Overall Framework Health: ✅ EXCELLENT

The framework demonstrates strong adherence to modern PHP principles:

• ✅ No inheritance (composition over inheritance)
• ✅ Extensive use of readonly classes
• ✅ Rich Value Object architecture
• ✅ Proper exception hierarchy
• ✅ Clean dependency injection

Priority 1: Critical Refactorings

1.1 MagicLinks: Eliminate Primitive Array Obsession

Issue: Commands and Value Objects use primitive array types instead of Value Objects

Current State:

// ❌ Primitive obsession in Commands
final readonly class GenerateMagicLinkCommand
{
    public function __construct(
        public TokenAction $action,
        public array $payload,              // ⚠️ Primitive array
        public ?TokenConfig $config = null,
        public ?string $baseUrl = null,
        public ?string $createdByIp = null,
        public ?string $userAgent = null
    ) {}
}

final readonly class ExecuteMagicLinkCommand
{
    public function __construct(
        public MagicLinkToken $token,
        public array $context = []          // ⚠️ Primitive array
    ) {}
}

// ❌ Primitive arrays in Value Objects
final readonly class MagicLinkData
{
    public function __construct(
        public array $payload,              // ⚠️ Primitive array
        // ... other properties
    ) {}
}

final readonly class TokenConfig
{
    public function __construct(
        public array $allowedIpRanges = [], // ⚠️ Primitive array
        public array $metadata = []         // ⚠️ Primitive array
        // ... other properties
    ) {}
}

final readonly class ActionResult
{
    public function __construct(
        public array $data = [],            // ⚠️ Primitive array
        public array $errors = []           // ⚠️ Primitive array
    ) {}
}

Recommended Refactoring:

Create dedicated Value Objects:

// ✅ Create MagicLinkPayload Value Object
namespace App\Framework\MagicLinks\ValueObjects;

final readonly class MagicLinkPayload
{
    private function __construct(
        private array $data
    ) {
        if (empty($data)) {
            throw new \InvalidArgumentException('Payload cannot be empty');
        }
    }

    public static function fromArray(array $data): self
    {
        return new self($data);
    }

    public function toArray(): array
    {
        return $this->data;
    }

    public function get(string $key, mixed $default = null): mixed
    {
        return $this->data[$key] ?? $default;
    }

    public function has(string $key): bool
    {
        return isset($this->data[$key]);
    }

    public function with(string $key, mixed $value): self
    {
        $data = $this->data;
        $data[$key] = $value;
        return new self($data);
    }
}

// ✅ Create ExecutionContext Value Object
namespace App\Framework\MagicLinks\ValueObjects;

final readonly class ExecutionContext
{
    private function __construct(
        private array $data
    ) {}

    public static function empty(): self
    {
        return new self([]);
    }

    public static function fromArray(array $data): self
    {
        return new self($data);
    }

    public function with(string $key, mixed $value): self
    {
        $data = $this->data;
        $data[$key] = $value;
        return new self($data);
    }

    public function toArray(): array
    {
        return $this->data;
    }
}

// ✅ Create IpRangeCollection Value Object
namespace App\Framework\MagicLinks\ValueObjects;

final readonly class IpRangeCollection
{
    /** @param array<string> $ranges */
    private function __construct(
        private array $ranges
    ) {
        foreach ($ranges as $range) {
            if (!$this->isValidIpRange($range)) {
                throw new \InvalidArgumentException("Invalid IP range: {$range}");
            }
        }
    }

    public static function fromArray(array $ranges): self
    {
        return new self($ranges);
    }

    public static function empty(): self
    {
        return new self([]);
    }

    public function contains(string $ip): bool
    {
        foreach ($this->ranges as $range) {
            if ($this->ipInRange($ip, $range)) {
                return true;
            }
        }
        return false;
    }

    private function isValidIpRange(string $range): bool
    {
        // Validation logic
        return true;
    }

    private function ipInRange(string $ip, string $range): bool
    {
        // Range checking logic
        return true;
    }
}

// ✅ Create Metadata Value Object
namespace App\Framework\MagicLinks\ValueObjects;

final readonly class Metadata
{
    private function __construct(
        private array $data
    ) {}

    public static function empty(): self
    {
        return new self([]);
    }

    public static function fromArray(array $data): self
    {
        return new self($data);
    }

    public function with(string $key, mixed $value): self
    {
        $data = $this->data;
        $data[$key] = $value;
        return new self($data);
    }

    public function toArray(): array
    {
        return $this->data;
    }
}

// ✅ Create ActionResultData Value Object
namespace App\Framework\MagicLinks\ValueObjects;

final readonly class ActionResultData
{
    private function __construct(
        private array $data
    ) {}

    public static function empty(): self
    {
        return new self([]);
    }

    public static function fromArray(array $data): self
    {
        return new self($data);
    }

    public function toArray(): array
    {
        return $this->data;
    }
}

// ✅ Create ErrorCollection Value Object
namespace App\Framework\MagicLinks\ValueObjects;

final readonly class ErrorCollection
{
    /** @param array<string> $errors */
    private function __construct(
        private array $errors
    ) {}

    public static function empty(): self
    {
        return new self([]);
    }

    public static function fromArray(array $errors): self
    {
        return new self($errors);
    }

    public function add(string $error): self
    {
        $errors = $this->errors;
        $errors[] = $error;
        return new self($errors);
    }

    public function hasErrors(): bool
    {
        return !empty($this->errors);
    }

    public function toArray(): array
    {
        return $this->errors;
    }
}

// ✅ Updated Command Classes
namespace App\Framework\MagicLinks\Commands;

use App\Framework\MagicLinks\ValueObjects\MagicLinkPayload;
use App\Framework\MagicLinks\TokenAction;
use App\Framework\MagicLinks\TokenConfig;

final readonly class GenerateMagicLinkCommand
{
    public function __construct(
        public TokenAction $action,
        public MagicLinkPayload $payload,     // ✅ Value Object
        public ?TokenConfig $config = null,
        public ?string $baseUrl = null,
        public ?string $createdByIp = null,
        public ?string $userAgent = null
    ) {}
}

use App\Framework\MagicLinks\ValueObjects\ExecutionContext;
use App\Framework\MagicLinks\MagicLinkToken;

final readonly class ExecuteMagicLinkCommand
{
    public function __construct(
        public MagicLinkToken $token,
        public ExecutionContext $context      // ✅ Value Object
    ) {}

    public static function withToken(MagicLinkToken $token): self
    {
        return new self($token, ExecutionContext::empty());
    }
}

// ✅ Updated MagicLinkData
namespace App\Framework\MagicLinks;

use App\Framework\MagicLinks\ValueObjects\MagicLinkPayload;

final readonly class MagicLinkData
{
    public function __construct(
        public string $id,
        public TokenAction $action,
        public MagicLinkPayload $payload,     // ✅ Value Object
        public DateTimeImmutable $expiresAt,
        public DateTimeImmutable $createdAt,
        public bool $oneTimeUse = false,
        public ?string $createdByIp = null,
        public ?string $userAgent = null,
        public bool $isUsed = false,
        public ?DateTimeImmutable $usedAt = null
    ) {}
}

// ✅ Updated TokenConfig
namespace App\Framework\MagicLinks;

use App\Framework\MagicLinks\ValueObjects\IpRangeCollection;
use App\Framework\MagicLinks\ValueObjects\Metadata;

final readonly class TokenConfig
{
    public function __construct(
        public int $ttlMinutes = 60,
        public bool $oneTimeUse = false,
        public IpRangeCollection $allowedIpRanges, // ✅ Value Object
        public Metadata $metadata,                  // ✅ Value Object
        public ?int $maxUsageCount = null,
        public bool $requireUserAgent = false
    ) {}

    public static function default(): self
    {
        return new self(
            allowedIpRanges: IpRangeCollection::empty(),
            metadata: Metadata::empty()
        );
    }
}

// ✅ Updated ActionResult
namespace App\Framework\MagicLinks\Actions;

use App\Framework\MagicLinks\ValueObjects\ActionResultData;
use App\Framework\MagicLinks\ValueObjects\ErrorCollection;

final readonly class ActionResult
{
    public function __construct(
        public bool $success,
        public ?string $message = null,
        public ActionResultData $data,        // ✅ Value Object
        public ErrorCollection $errors,       // ✅ Value Object
        public ?string $redirectUrl = null
    ) {}

    public static function success(
        string $message = null,
        ActionResultData $data = null
    ): self {
        return new self(
            success: true,
            message: $message,
            data: $data ?? ActionResultData::empty(),
            errors: ErrorCollection::empty()
        );
    }

    public static function failure(
        string $message,
        ErrorCollection $errors = null
    ): self {
        return new self(
            success: false,
            message: $message,
            data: ActionResultData::empty(),
            errors: $errors ?? ErrorCollection::empty()
        );
    }
}

Migration Strategy:

1. Create Value Objects in /src/Framework/MagicLinks/ValueObjects/
2. Update Commands to use Value Objects
3. Update MagicLinkData, TokenConfig, ActionResult
4. Update handlers to work with new Value Objects
5. Add factory methods for backward compatibility during transition
6. Update tests to use new Value Objects
7. Update documentation with new patterns

Impact:

• ✅ Better type safety
• ✅ Self-documenting code
• ✅ Validation at construction
• ✅ Immutable transformations
• ✅ Consistent with framework patterns

Files to Modify:

• /src/Framework/MagicLinks/Commands/GenerateMagicLinkCommand.php
• /src/Framework/MagicLinks/Commands/ExecuteMagicLinkCommand.php
• /src/Framework/MagicLinks/MagicLinkData.php
• /src/Framework/MagicLinks/TokenConfig.php
• /src/Framework/MagicLinks/Actions/ActionResult.php
• /src/Framework/MagicLinks/Commands/GenerateMagicLinkHandler.php
• /src/Framework/MagicLinks/Commands/ExecuteMagicLinkHandler.php

1.2 Error/Exception Module Consolidation

Issue: Framework has 5 separate error-related modules with potential overlap

Current Modules:

• ErrorAggregation/ - Error event aggregation and pattern detection
• ErrorBoundaries/ - Resilience patterns (retry, circuit breaker)
• ErrorHandling/ - Core error handling and logging
• ErrorReporting/ - Error reporting and analytics
• Exception/ - Base exception classes and error codes

Concern: Potential functional overlap and confusion about where error-related code belongs

Recommended Analysis:

1. Audit Module Responsibilities:

   # Analyze each module's actual responsibilities
   # Check for duplicate functionality
   # Identify cross-module dependencies
   

2. Potential Consolidation (after audit):

   • Keep Exception/ as base exception framework
   • Keep ErrorBoundaries/ for resilience patterns (distinct purpose)
   • Consider merging ErrorHandling/, ErrorAggregation/, and ErrorReporting/ into single ErrorManagement/ module with clear sub-namespaces:
     • ErrorManagement/Handling/ - Core error handling
     • ErrorManagement/Aggregation/ - Pattern detection
     • ErrorManagement/Reporting/ - Analytics and reporting

3. Decision Criteria:

   • Do modules have genuinely separate concerns?
   • Is there code duplication across modules?
   • Would consolidation improve or harm discoverability?

Action: Detailed audit required before implementation

Priority 2: Consistency Improvements

2.1 Naming Convention Standardization

Issue: Inconsistent naming for similar components

Current Pattern:

Queue/Services/ProgressManager.php
Queue/Services/JobMetricsManager.php
Queue/Services/JobCleanupService.php

Recommendation: Establish clear naming conventions

Proposed Standard:

• Manager: Coordinates multiple operations, manages lifecycle
• Service: Performs specific business operations
• Handler: Handles commands/events/requests
• Processor: Processes/transforms data
• Builder: Constructs complex objects
• Factory: Creates instances
• Repository: Data access abstraction
• Registry: Maintains collections of items

Example Application:

✅ JobLifecycleManager - manages job lifecycle
✅ JobCleanupService - specific cleanup operations
✅ JobMetricsCollector - collects metrics
✅ DeadLetterManager - manages dead letter queue

2.2 Command/Handler Consistency

Current State: Generally good, but verify all Commands have corresponding Handlers

Verification Steps:

# Find all Command classes
find src/Framework -name "*Command.php" | sort

# Find all Handler classes
find src/Framework -name "*Handler.php" | sort

# Ensure 1:1 mapping

Recommendation: Enforce convention where every XyzCommand has XyzHandler

Priority 3: Architecture Enhancements

3.1 Value Object Directory Structure

Current State: 49 Value Object directories across framework - EXCELLENT!

Recommendation: Create Value Object documentation/index

Suggested Documentation:

# Framework Value Objects

## Core Value Objects (Framework/Core/ValueObjects/)
- `Duration` - Time duration representation
- `Timestamp` - Point in time
- `Url` - URL validation and manipulation
- `EmailAddress` - Email validation
- `Money` - Monetary values with currency
- `ClassName` - Fully qualified class names
- `Hash` - Hash values with algorithm
- `Port` - Network port numbers
- `Percentage` - Percentage values (0-100)

## Domain-Specific Value Objects
[Auto-generate from discovery...]

## Usage Guidelines
- Always prefer Value Objects over primitives for domain concepts
- Value Objects should be `final readonly`
- Include validation in constructor
- Provide factory methods for common cases
- Implement transformation methods that return new instances

3.2 Module Inter-Dependencies

Recommendation: Create dependency graph visualization

Tool Suggestion:

// Create ModuleDependencyAnalyzer
namespace App\Framework\Quality;

final readonly class ModuleDependencyAnalyzer
{
    public function analyzeModuleDependencies(): array
    {
        // Scan use statements across modules
        // Build dependency graph
        // Detect circular dependencies
        // Generate visualization
    }

    public function detectCircularDependencies(): array
    {
        // Identify circular dependencies
    }

    public function calculateCouplingMetrics(): array
    {
        // Calculate afferent/efferent coupling
        // Identify highly coupled modules
    }
}

Priority 4: Performance Optimizations

4.1 Initializer Optimization

Current State: 65 Initializer classes

Recommendation: Analyze and optimize initializer execution order

Suggested Tool:

namespace App\Framework\DI;

final readonly class InitializerPerformanceAnalyzer
{
    public function analyzeInitializerPerformance(): array
    {
        // Track initialization time for each Initializer
        // Identify slow initializers
        // Suggest optimization opportunities
        // Detect unnecessary initialization in CLI context
    }

    public function suggestLazyInitialization(): array
    {
        // Suggest which services could be lazy-loaded
    }
}

4.2 Service Interface Optimization

Current State: 24+ service interfaces

Recommendation: Ensure interfaces are truly necessary

Analysis Criteria:

• Does interface have multiple implementations?
• Is interface used for dependency injection?
• Does interface provide abstraction value?
• Could concrete class suffice?

Tool:

# Find interfaces with single implementation
for interface in $(find src/Framework -name "*Interface.php" -o -name "*Service.php" | grep interface); do
    implementations=$(grep -r "implements $(basename $interface .php)" src/ | wc -l)
    if [ $implementations -eq 1 ]; then
        echo "Single implementation: $interface"
    fi
done

Priority 5: Testing Improvements

5.1 Test Coverage for Value Objects

Recommendation: Ensure 100% test coverage for all Value Objects

Suggested Test Pattern:

describe('MagicLinkPayload', function () {
    it('validates non-empty payload on construction', function () {
        expect(fn() => MagicLinkPayload::fromArray([]))
            ->toThrow(\InvalidArgumentException::class);
    });

    it('provides immutable transformations', function () {
        $payload = MagicLinkPayload::fromArray(['key' => 'value']);
        $updated = $payload->with('new_key', 'new_value');

        expect($payload)->not->toBe($updated);
        expect($updated->toArray())->toHaveKey('new_key');
        expect($payload->toArray())->not->toHaveKey('new_key');
    });

    it('safely retrieves values with defaults', function () {
        $payload = MagicLinkPayload::fromArray(['key' => 'value']);

        expect($payload->get('key'))->toBe('value');
        expect($payload->get('missing', 'default'))->toBe('default');
    });
});

5.2 Integration Test Coverage

Recommendation: Verify integration between MagicLinks and other modules

Key Integration Points:

• CommandBus integration
• Cache service integration
• Event system integration (if applicable)
• Action registry discovery

Implementation Priority

Phase 1 (High Priority - 1-2 weeks):

1. ✅ MagicLinks Value Object refactoring (Priority 1.1)
2. ✅ Naming convention audit and standardization (Priority 2.1)
3. ✅ Command/Handler consistency verification (Priority 2.2)

Phase 2 (Medium Priority - 2-4 weeks):

1. ✅ Error module consolidation analysis (Priority 1.2)
2. ✅ Value Object documentation (Priority 3.1)
3. ✅ Module dependency analysis (Priority 3.2)

Phase 3 (Lower Priority - 1-2 months):

1. ✅ Initializer performance optimization (Priority 4.1)
2. ✅ Service interface optimization (Priority 4.2)
3. ✅ Test coverage improvements (Priority 5)

Success Metrics

Code Quality Metrics:

• ✅ Reduce primitive array usage in Commands by 100%
• ✅ Achieve 100% Value Object test coverage
• ✅ Reduce module coupling by 20%
• ✅ Standardize 95%+ of naming conventions

Performance Metrics:

• ✅ Reduce initialization time by 15%
• ✅ Identify and eliminate circular dependencies

Developer Experience:

• ✅ Clear module responsibilities documentation
• ✅ Value Object usage guidelines
• ✅ Dependency graph visualization

Conclusion

The Custom PHP Framework demonstrates excellent architectural principles with strong adherence to:

• Composition over inheritance
• Immutability with readonly classes
• Rich Value Object usage
• Clean dependency injection

Primary Focus Areas:

1. Eliminate primitive array obsession in MagicLinks module
2. Standardize naming conventions across services/managers/handlers
3. Document and visualize Value Object architecture
4. Analyze and potentially consolidate error-related modules

The framework is in excellent health and these refactorings will enhance an already solid architecture.