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

Question

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

Answer 1

You don't need the factory pattern for what you are doing from what I can tell as rather than different types of image and product classes, you only have one product class with different details.

I suggest creating a single product class with a constructor that receives a single row of information from the product database and a collection of images that belong to it. The constructor can then setup the product class.

Then, create a collection or array of products in the product repo class and return that.

Something like this (written in pseudo-php)

    Class Product
    {
        public function __construct(productInfo, imageArray)
        {
            //contruct product here
        }
    }

    Class ProductRepo
    {

        public function getProducts()
        {
            //retrieve products
            $items = getProducts();

            //setup products
            return setupProducts($items);
        }

        private function setupProducts($items)
        {

            foreach($items as $item){
                $images = $this->getImages($product->getId());

                $product = new Product($item, $images);

                $products[] = $product;
        }
            return $products;
        }

        private function getImages($itemId)
        {
            //get and return images for this product
        }

        private function loadProducts()
        {
            //load from database and return all products
        }
    }

The factory pattern is for situations where you need multiple implementations of an interface with different functionality in the concrete objects and you need a way to select the right one. For example, if you have an application where you trying to calculate the area of various shapes, you might have an IShapes interface with a calculateArea() function and several classes that implement it (For example, Circle, Triangle, Rectangle, etc) with all use different formula's to calculate the area of the shape. You could then use a factory to construct and fetch the correct implementation for a particular shape name for a common set of parameters.

edit: If there is something functionally different between different product types, say, the way reward points are calculated, you could do something like this:

    class ProductFactory
    {
        public Iproduct getProduct($productType, $productInfo, $images)
        {
            switch(productType)
            {
                case: featured
                    return new featuredProduct($productInfo)
                case: standard
                    return new standardProduct($productInfo)
            }
        }
    }

    Interface Iproducts
    {
        //suppose different product types have different reward point formula's
        calculateRewardPoints();

        ....
        //other functions
    }

Which could then be used in the product repo above like so:

    private function setupProducts($items)
    {
        foreach($items as $item){
        $images = $this->getImages($product->getId());

            $product = ProductFactory.getProduct($item.type, $item, $images);

            $products[] = $product;
    }

Answer 2

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