How to properly use models inside models?

[eluser]paulodeleo[/eluser]
Hi,

I started using codeignitor a few days ago, and noticed something that I don't know if is caused by some mistake of mine (just wrong design being used), or a limit/bug in php or codeignitor .

The problem is when I need to use models inside models, and return an array of this model objects to the controller. Looks like the "submodels" loose their references. To the controller, all "submodels" are the same, the last instance of the "submodel".

Sorry for my english. Maybe the best way to explain is with some sample code of the problem.

fruit.php (model, the one I call "submodel"):

Code:

<?php
class Fruit extends Model {
    
    public $id;
    
    function __construct()
    {
        parent::Model();
    }
    
}
?>

tree.php (model, buggy)

Code:

<?php
class Tree extends Model {
    
    public $id;
    
    function __construct()
    {
        parent::Model();
        $this->load->model("fruit");
    }
    
    function load($id)
    {
        $this->id = $id;
        $this->fruit->id = $id;
    }
    
    function listAll()
    {
        $objArray = array();
        for($i=0; $i<=3; $i++)
        {
            $obj = new Tree();
            $obj->load($i);
            $objArray[]=$obj;
        }
        return $objArray;
    }
    
}
?&gt;

test.php (controller):

Code:

<?php
class Test extends Controller {

    function index()
    {
        $this->load->model("tree");
        foreach ($this->tree->listAll() as $tree)
        {
            echo "tree: " . $tree->id . ", fruit: " . $tree->fruit->id . "<br>";
        }
    }
    
}
?&gt;

Browser output (buggy version):

Code:

tree 0, fruit 3
tree 1, fruit 3
tree 2, fruit 3
tree 3, fruit 3

The problem is all fruits to show id 3, when I expect the same id as it's tree.
To fix it, I must do some changes:

tree.php (model, fixed with workaround)

Code:

<?php
//1 - Must require the fruit class...
require_once("fruit.php");
class Tree extends Model {
    
    public $id;
    //2 - Create a public property for the fruit object...
    public $fruit;
    
    function __construct()
    {
        parent::Model();
        
        //3 - Remove CI's loading of the model and use the native php one
        //$this->load->model("fruit");
        $this->fruit = new Fruit();
    }
    
    function load($id)
    {
        $this->id = $id;
        $this->fruit->id = $id;
    }
    
    function listAll()
    {
        $objArray = array();
        for($i=0; $i<=3; $i++)
        {
            $obj = new Tree();
            $obj->load($i);
            $objArray[]=$obj;
        }
        return $objArray;
    }
    
}
?&gt;

Expected browser output (fixed by workaround):

Code:

tree 0, fruit 0
tree 1, fruit 1
tree 2, fruit 2
tree 3, fruit 3

Then the output is right. But you can imagine how ugly (and wrong?) the code can be in a real world situation. Lots of requires in every model in the "chain", etc.

So, if the code is wrong in some way, how codeignitor must be used to produce the expected output?
Or, if it's really some kind of bug, any other suggestions to solve it?
Using PHP 5.2.6 on a Linux box here.

Thank's for any help!

[eluser]Colin Williams[/eluser]
I don't see anything wrong with your workaround. CI's model implementation certainly isn't structured to be used in the manner you are here, so you do have to go outside the box a little.

[eluser]paulodeleo[/eluser]
Thanks for the fast answer Colin (sorry for the lack of mine).
So, isn't CI the right tool to be used if you need to have stuff like "$tree->fruit->id" in php?
Should the way it works be treated really as a bug, 'cause otherwise it should trigger a warn or error when used that way, right?

[eluser]Randy Casburn[/eluser]
@paulodeleo -- This can work fine. You simply have a logic error in your code. There is no need for the second model file named Fruit since you are dynamically creating that object on the fly. Your Test controller stays exactly the same.

I put in the exception handling so you could play with the differences and see what was going on. Your code was throwing and exception immediately upon trying to access the non-existent Object named 'fruit' the first time through. Subsequent trips through became very interesting based upon the PHP version and error_lvl that was set for PHP. This was a fun one look at, thanks.

Try this and you'll see that it works fine as you first wanted it to:

Code:

class Tree extends Model {
    
    public $id;
    public $fruit;
    
    
    public function __construct()
    {
        parent::Model();
    }
    
    private function load($id)
    {
        $this->id = $id;

        if (!is_object($this->fruit)) {
            $this->fruit = (object) $this->fruit;
        }    
        if (!is_object($this->fruit)) {
            throw new Exception('No Fruit Object');
        }

        try {
            $this->fruit->id = $id;
        } catch (Exception $e) {
            echo 'Caught exception: ',  $e->getMessage(), "\n";
        }

   }
    
    public function listAll()
    {
        $objArray = array();
        for($i=0; $i<=3; $i++)
        {
            $obj = new Tree();
            $obj->load($i);
            $objArray[]=$obj;
        }
        return $objArray;
    }
    
}

Hope this is helpful,

Randy

[eluser]paulodeleo[/eluser]
Thank you too Randy, good to see this level of support here.

I really need to make it work having a separated model class for fruit. The idea is to be able to use something like "$person->house->tree->fruit->id", the same way as "$tree->fruit->id", and all objects being made of database data loaded in the load() method, in a real world application.

Well, I don't really understand what happened with the code sections of my first post (was edited by the staff?). Line 11 of the first tree.php (the buggy one) is supposed to be:

Code:

$this->load->model("fruit");

just as it's commented on the second (the fixed one). I edited it so it stays the way I originally meant to.

That's why I haven't any error here like the one you treated. So, the fruit object of the tree object is aways there, loaded in the constructor. So, no logic error, no need for exception handling. But I kept it in the new version above anyway. And now I found another, simpler, way to make it work:

tree.php (model, fixed with workaround version 2)

Code:

<?php
class Tree extends Model {
    
    public $id;
    public $fruit;
    
    
    public function __construct()
    {
        parent::Model();
        $this->load->model("fruit");
        //Now something interesting: adding the above line everything work as intended
        //(re-set the fruit object passing by reference. if the "&" is ommited, the "bug" remains)
        $this->fruit = & new Fruit();
    }
    
    private function load($id)
    {
        $this->id = $id;

        if (!is_object($this->fruit)) {
            throw new Exception('No Fruit Object');
        }

        try {
            $this->fruit->id = $id;
        } catch (Exception $e) {
            echo 'Caught exception: ',  $e->getMessage(), "\n";
        }

   }
    
    public function listAll()
    {
        $objArray = array();
        for($i=0; $i<=3; $i++)
        {
            $obj = new Tree();
            $obj->load($i);
            $objArray[]=$obj;
        }
        return $objArray;
    }
    
}
?&gt;

So lets rollback: isn't the problem really the way CI loads a model inside another model (not passing by reference)? Can you take another look at it?

[eluser]Randy Casburn[/eluser]
Yes I saw load->fruit was in the second and not the first and wondered about that. As for the rest, I we may have a bit of a problem. CI uses a bit of a Registry Pattern (whether it is realized or not I don't know) to maintain and keep track of all the instantiated classes. I'm not sure CI will allow you to instantiate a second object of the same class. From a pure programming point of view, I know that is just plain silly. Let me look and see if I'm correct.

Randy

[eluser]Randy Casburn[/eluser]
[quote author="paulodeleo" date="1219363978"]

Code:

public function listAll()
    {
        $objArray = array();
        for($i=0; $i<=3; $i++)
        {
            $obj = new Tree();
            $obj->load($i);
            $objArray[]=$obj;
        }
        return $objArray;
    }
    
}
?&gt;

So lets rollback: isn't the problem really the way CI loads a model inside another model (not passing by reference)? Can you take another look at it?[/quote]

I'm not really sure that it is. I appreciate the efficiency of your coding method, yet you'll find it won't integrate well with either PHP or CI. As an example of what I mean, I left the snippet of code above because it was interesting to me from the beginning. This isn't really the proper way to create exact duplicates of objects in PHP regardless of the framework. It works, but doesn't integrate well because it violates so many "conventions" found in PHP. The proper way to create exact copies of object, when you want the methods and the members intact is to use the clone key word with your object. It accomplishes for you what your attempting here.

So that's one of the PHP stories I think. Another PHP story that is more closely related to CI convention is that of chaining objects. Since PHP multi-inheritance is not supported by PHP, the work around is to chaining extended classes together. Hard core OOP developers throw up in their mouth about right now when they realize what what just said. But those are the conventions and you're violating them in very subtle ways.

Perhaps Action Script 3.0 is more suited to your style of coding.

Sorry if this irritates you. You've certainly done nothing wrong and have demonstrated quite a prowess.

As I've demonstrated, you can make "your way" work. I don't think I'm interested in bending PHP or CI in that regard. And I'm certain I don't completely understand what it is you're attempting to accomplish either. So please keep that in mind.

Randy

[eluser]wiredesignz[/eluser]
Since this is NOT multiple inheritance let me offer a more simple explanation of what is happening.

When CI loads a model (tree) it assigns a reference of all the current controller class variables to that model.

Your second model (fruit) then contains a reference to the first (tree) model.

After the tree class constructor completes the tree object is assigned a reference to fruit by CI's model class. (singleton pattern)

Any subsequent (tree) model instances are assigned a reference to the original (fruit) model

So after your list_all() method runs, you have many tree objects but they all share the same fruit object, hence $fruit->id = 3

Your workaround creating more fruit instances is a solution but it's not very elegant.

[eluser]Randy Casburn[/eluser]
[quote author="wiredesignz" date="1219381748"]Since this is NOT multiple inheritance let me offer a more simple explanation of what is happening.[/quote]

I never said it was. In fact, I said PHP won't do multiple inheritance.

[quote author="wiredesignz" date="1219381748"]When CI loads a model (fruit) it assigns a reference of all the current controller class variables to that model.[/quote]

You state this absolutely and you are wrong. It makes a choice as to whether references will be used based upon the argument sent to the function that does this. Then, it uses an assignment statement to either make a direct assignment or make a referenced assignment of the every CI Object (controller) member.

Code:

if ($use_reference == TRUE)
    {
        // Needed to prevent reference errors with some configurations
        $this->$key = '';
        $this->$key =& $CI->$key;
    }
    else
    {
        $this->$key = $CI->$key;
    }

[quote author="wiredesignz" date="1219381748"]Your second model (tree) then contains a reference to the first (fruit) model.

Any sunsequent (tree) model instances are assigned a reference to the original (fruit) model.[/quote]

If you were more interested in helping and less interested in challenging and nit-picking you would have noticed that, not only your mistake about concerning the multiple inheritance thing, but you got the order of instantiation wrong as well.

[quote author="wiredesignz" date="1219381748"]Your workaround creating more fruit instances is a solution but it's not very elegant.[/quote]

This was the conclusion of the previous post wiredesignz. The nature of PHP and CI indicate this OP should do things differently. It can be done however the OP pleases, but I as much as acknowledged that my 'inelegant' solution was not proper and I didn't see any benefit in going any further with it.

If you have something positive to say...some code to add, some constructive thoughts for this particular OP...great.

Thanks for your insight.

Randy

[eluser]wiredesignz[/eluser]
Randy, read the model code. Firstly the class checks for __get and __set (PHP5) then decides wether or not to use the reference indicator, and you know PHP5 assigns by refernece by default anyway so either way its still by reference.

So pull your head in my friend. After which your apology will be accepted

EDIT:
Order of instantiation is correct? Please stop misquoting my other (edited) post.