Search code examples
design-patternssolid-principlescyclic-dependency

How to break and improve cyclic dependency without using proxy pattern?


My classes are depending upon too many other classes and I couldnot find ways to improve it. The problem goes something like below:

I have a ProductRepo, ProductFactory and a ImageFactory classes. ProductRepo does the db thing on products table and fetches rows as array. This array is passed to ProductFactory to create a Product Modal. Product modals also has images linked to it.

client code:

$products = $this->productRepo->findAll('...');
foreach($products as $product){
    ...
    //get images
    $images = $product->getImages();
    ...
}

Class ProductRepo implements ProductRepositoryInterface{
    protected $productFactory;
    protected $imageFactory;
    public function __construct(ProductFactoryInterface $productFactory, ImageFactoryInterface $imageFactory)
    {
        $this->productFactory = $productFactory;
        $this->imageFactory = $imageFactory;
    }

    public function findAll(...)
    {
        $result = $this->execute('....');
        $products = $this->productFactory->make($result);
        return $products;
    }

    public function getImages($productId)
    {
        $result = $this->execute('....');
        $images = $this->imageFactory->make($result);
        return $images;
    }
}

Class ProductFactory implements ProductFactoryInterface{
    protected $productRepo;
    public function __construct(ProductRepositoryInterface $productRepo)
    {
        $this->productRepo = $productRepo;
    }

    public function make($items)
    {
        ...
        $products = [];
        foreach($items as $item){
            $product = new Product($item);
            $item->setImages($this->productRepo->getImages($product->getId()));
            $products[] = $product;
        }
        ...
        return $products;
    }
}

Class ImageFactory implements ImageFactoryInterface{
    public function make($items)
    {
        ...
        $images = [];
        foreach($items as $item){
            $image = new Image($item);
            $images[] = $image;
        }
        ...
        return $images;
    }
}

So, I have following problems:

  1. cyclic dependency ProductRepo --> ProductFactory --> ProductRepo

    To skip this, I can use a setter injection or use a proxy pattern. But I think that would not a good solution. How do you guys handle this kind of problems?

  2. ProductRepo depends on both the ProductFactory and ImageFactory. Is this a good practise to depend on more than one factory?

I think the problems are clear. :) Thank you


Solution

  • There are several ways to break the cyclic dependency, but the most funadmental problem seems to be that ProductFactory requries a ProductRepo, which must itself be able to construct products, even though this functionality will not be used and it probably doesn't make sense to pass a ProductRepo that uses a different factory (hidden rule). So:

    1) Make an ImageRepositoryInterface that has only the getImages method. ProductRepositoryInterface can either extend this interface or ProductRepo can implement it independently. Then, pass the image repository into ProductFactoryInterface.make instead of requiring it on construction. You can pass your ProductRepo at this time.

    2) Yes, there's no problem with depending on more than one kind of factory