Search code examples
phpphp-7.4

Infinty loop in constuctor - Creating the same istances of related objects


I got stuck in an endless loop creating the same objects over and over in the models.

But from the beginning:

I have two related models, lets call A and B

A could has many B and B belongs to one A.

And since model B refers to the fields of A, I added it as protected property to avoid creating new object when calling B method related with A fields.

The constuctor of B looks like:

public function __construct(int $id = null)
{
    parent::__construct($id);

    $a = $this->get('a_id'); 
    if ($a) {
        $this->a = new A($a);
    }
}

and A:

public function __construct(int $id = null)
{
    parent::__construct($id);

    $this->date = new CarbonPL($this->get('date'));
    $this->initB();
}

But then I realized that calling initB() creates again the same instance o B with creating the same A and so on forever

private function initB()
{
    if (!$this->isReferenced()) { // Checks wheter is instance exists in DB
        return;
    }
    $query = B::getIDQuery();
    $query .= ' WHERE is_del IS FALSE';
    $query .= ' AND a_id = ' . $this->id;

    $ids = Helper::queryIds($query);

    foreach ($ids as $id) {
        $this->B[] = new B($id);
    }
}

I need that object of A contains loaded of B and vice verse, because a lot of B refer to A field and vice versa, but how to prevent yourself from such an endless loop.

I quickly got the idea of an extra parameter in B which would be A, and if it were, I wouldn't have to reuse the constuctor. But I don't really like this solution because of the second parameter and I was wondering how to solve it in a different (better) way, preferably so that you can still only enter the identifier.

B conctructor after that fix:

public function __construct(int $id = null, A $a = null)
{
    parent::__construct($id);

    if ($a) {
        $this->a = $a;
    } else {
        $a = $this->geti('a_id');
        if ($a) {
            $this->a = new A($a);
        }
    }
}

Solution

  • There's nothing wrong with the 2 parameter solution you came up with but if you're looking for another option...

    One way to deal with this is to create a cache of instances for each class so that when someone needs an instance of your class they don't call new() they call a factory method that will construct a new instance only if there is not one already in the cache with the ID.

    <?php
    
    class A {
        private static $cache = array();
        
        private function __construct( $id ) {
            // A private constructor prevents any other class from creating new instances
            // It could be protected if you want to allow subclasses to call new A()
            parent::__construct( $id );
            $this->date = new CarbonPL($this->get('date'));
            $this->initB();
        }
        
        public static function create_for_id( $id ) {
            if ( isset( self::$cache[ $id ] ) ) {
                $result = self::$cache[ $id ];
            } else {
                $result = new A( $id );
                self::$cache[ $id ] = $result; // Save it for later reuse
            }
            return $result;
        }
    }
    

    In class B you can do the same thing or however it will work best for your situation. Now in your B constructor (or anywhere you want a new A) rather than calling new A($id) you will call A::create_for_id( $id ). If an A with that ID already exists then it will reuse it rather than constructing endless new instances.