DI Review

DI Review Areas

Dependency Injection Review:
├── Constructor Injection
│   ├── All dependencies injected
│   ├── No object manager usage
│   ├── Proper type hints
│   └── Required vs optional dependencies
├── Interface Binding
│   ├── Interfaces for all services
│   ├── Proper interface binding
│   ├── No concrete class dependencies
│   └── Interface segregation
├── Virtual Types
│   ├── Proper configuration
│   ├── No overuse
│   └── Clear purpose
└── Shared Objects
    ├── Proper sharing configuration
    ├── Stateless services
    ├── Thread safety
    └── Memory implications

Review Checklist

$diChecklist = [
    'constructor' => [
        'All dependencies injected via constructor',
        'No ObjectManager usage',
        'Proper type hints',
        'Required dependencies first'
    ],
    'interfaces' => [
        'Services have interfaces',
        'Dependencies on interfaces',
        'No concrete class dependencies',
        'Interface segregation principle'
    ],
    'virtual_types' => [
        'Clear purpose for virtual type',
        'Not overused',
        'Properly configured'
    ],
    'shared' => [
        'Stateless services shared',
        'Stateful services not shared',
        'Memory implications considered'
    ]
];

Proper Constructor Injection

// Bad: ObjectManager usage
class ProductService
{
    public function getProduct($id)
    {
        $product = $this->_objectManager->create('Magento\Catalog\Model\Product');
        $product->load($id);
        return $product;
    }
}

// Good: Constructor injection
class ProductService
{
    private $productRepository;
    
    public function __construct(
        ProductRepositoryInterface $productRepository
    ) {
        $this->productRepository = $productRepository;
    }
    
    public function getProduct($id)
    {
        return $this->productRepository->getById($id);
    }
}

Required vs Optional Dependencies

// Good: Required dependencies first
class OrderService
{
    public function __construct(
        OrderRepositoryInterface $orderRepository, // Required
        PaymentInterface $payment, // Required
        LoggerInterface $logger = null // Optional
    ) {
        $this->orderRepository = $orderRepository;
        $this->payment = $payment;
        $this->logger = $logger ?: ObjectManager::getInstance()->get(LoggerInterface::class);
    }
}

// Bad: Optional before required
class OrderService
{
    public function __construct(
        LoggerInterface $logger = null, // Optional
        OrderRepositoryInterface $orderRepository // Required
    ) {
        // This causes issues with DI
    }
}

Type Hints

// Good: Interface type hints
class ProductService
{
    public function __construct(
        ProductRepositoryInterface $repository, // Interface
        LoggerInterface $logger // Interface
    ) {}
}

// Bad: Concrete class type hints
class ProductService
{
    public function __construct(
        MysqlProductRepository $repository, // Concrete
        FileLogger $logger // Concrete
    ) {}
}

// Check for type hint violations
function checkTypeHints($class)
{
    $reflection = new \ReflectionClass($class);
    $constructor = $reflection->getConstructor();
    
    if (!$constructor) {
        return [];
    }
    
    $violations = [];
    
    foreach ($constructor->getParameters() as $param) {
        if ($param->getTypeHint() && !interface_exists($param->getTypeHint())) {
            $violations[] = $param->getName();
        }
    }
    
    return $violations;
}

Check Interface Binding

<!-- di.xml -->
<config>
    <!-- Good: Interface binding -->
    <type name="Magento\Framework\Logger\Monolog\Logger">
        <plugin name="my_plugin" type="Vendor\Module\Plugin\LoggerPlugin"/>
    </type>
    
    <!-- Bad: Concrete class binding -->
    <type name="Magento\Framework\Logger\Monolog\Logger">
        <plugin name="my_plugin" type="Vendor\Module\Plugin\LoggerPlugin"/>
    </type>
</config>

Interface Segregation

// Bad: Fat interface
class ProductRepositoryInterface
{
    public function get($id);
    public function save($product);
    public function delete($product);
    public function search($query);
    public function export($format);
}

// Good: Segregated interfaces
class ProductReadInterface
{
    public function get($id);
    public function search($query);
}

class ProductWriteInterface
{
    public function save($product);
    public function delete($product);
}

class ProductExportInterface
{
    public function export($format);
}

No Concrete Dependencies

// Bad: Depends on concrete class
class OrderService
{
    private $mysqlOrderRepository;
    
    public function __construct(
        MysqlOrderRepository $repository // Concrete
    ) {
        $this->mysqlOrderRepository = $repository;
    }
}

// Good: Depends on interface
class OrderService
{
    private $orderRepository;
    
    public function __construct(
        OrderRepositoryInterface $repository // Interface
    ) {
        $this->orderRepository = $repository;
    }
}

// Configuration
<type name="Vendor\Module\Service\OrderService">
    <arguments>
        <argument name="repository" xsi:type="object">MysqlOrderRepository</argument>
    </arguments>
</type>

Virtual Type Usage

<!-- Good: Virtual type for specific configuration -->
<config>
    <virtualType name="CustomLogger" type="Magento\Framework\Logger\Monolog\Logger">
        <arguments>
            <argument name="name" xsi:type="string">custom_channel</argument>
            <argument name="handlers" xsi:type="array">
                <item name="stream" xsi:type="object">CustomStreamHandler</item>
            </argument>
        </arguments>
    </virtualType>
    
    <!-- Use virtual type -->
    <type name="Vendor\Module\Service\MyService">
        <arguments>
            <argument name="logger" xsi:type="object">CustomLogger</argument>
        </arguments>
    </type>
</config>

<!-- Bad: Overuse of virtual types -->
<!-- Multiple virtual types for same purpose -->
<!-- Unclear purpose -->

Virtual Type Best Practices

// When to use virtual types
$useCases = [
    'configuration' => 'Different configuration for same class',
    'testing' => 'Mock objects for testing',
    'multi-tenant' => 'Different implementations per tenant'
];

// When NOT to use virtual types
$avoidCases = [
    'simple_dependency' => 'Just use interface binding',
    'overuse' => 'Too many virtual types confuse DI',
    'unclear_purpose' => 'If you can\'t explain why, don\'t use'
];

// Review virtual types
function reviewVirtualTypes($config)
{
    $virtualTypes = $config->getVirtualTypes();
    $issues = [];
    
    foreach ($virtualTypes as $name => $type) {
        // Check for overuse
        if (count($virtualTypes) > 10) {
            $issues[] = 'Too many virtual types';
        }
        
        // Check for clear purpose
        if (!$this->hasClearPurpose($name, $type)) {
            $issues[] = "Virtual type $name lacks clear purpose";
        }
    }
    
    return $issues;
}

Shared vs Non-Shared

// Shared objects (default)
// Single instance shared across application
// Good for: Stateless services, factories, repositories

// Non-shared objects
// New instance for each injection
// Good for: Stateful objects, request-specific objects

// Configuration
<config>
    <!-- Make service non-shared -->
    <type name="Vendor\Module\Model\RequestSpecificObject" shared="false"/>
    
    <!-- Default is shared=true -->
    <type name="Vendor\Module\Service\StatelessService"/>
</config>

Stateful vs Stateless

// Bad: Stateful service shared
class CartService
{
    private $items = []; // State
    
    public function addItem($item)
    {
        $this->items[] = $item; // State changes
    }
}
// If shared, state persists across requests!

// Good: Stateful service not shared
class CartService
{
    private $items = []; // State
    
    public function addItem($item)
    {
        $this->items[] = $item;
    }
}
// Configure as shared="false"

// Good: Stateless service shared
class ProductService
{
    public function getProduct($id)
    {
        // No state, safe to share
        return $this->repository->get($id);
    }
}

Thread Safety

// Shared objects must be thread-safe
class Counter
{
    private $count = 0; // Problematic if shared
    
    public function increment()
    {
        $this->count++; // Race condition in multi-threaded
    }
}

// Solution: Use non-shared for stateful objects
// Or use atomic operations
\Cos\function(function() {
    $counter = $this->counterFactory->create(); // New instance
    $counter->increment();
});

Memory Implications

// Shared objects persist in memory
// Consider memory usage

// Bad: Large data shared
class LargeDataSet
{
    private $data = []; // Large array
    
    public function __construct()
    {
        $this->data = $this->loadLargeData(); // Loaded once, stays in memory
    }
}

// Good: Load on demand
class LargeDataSet
{
    public function getData()
    {
        // Load when needed, can be garbage collected
        return $this->loadLargeData();
    }
}