Auto-Modeler icon indicating copy to clipboard operation
Auto-Modeler copied to clipboard

State problem

Open ccazette opened this issue 14 years ago • 2 comments

There is an issue with how states behaves. Luckily, it is easy to fix. This is the problem :

AutoModeler::factory("user", 1);

Will actually load the object. So the state is initially set to NEW, then to LOADING in the load method, then LOADED. That makes complete sense.

AutoModeler::factory("user")->load(NULL, 3)->current();

Will create the object from mysql_query_object. So the state is initially NEW, then direcly set to LOADED when the object is set. No LOADING.

This is especially annoying as mysql_query_object uses the magic __set(), whereas, load directly se the data in the _data array. So if in your magic set you want to do checks if the object is loading or if it is a manual set, you cannot rely on states...

The fix is simple : default AutoModeler state to STATE_LOADING, then in the construct method add an "else" statement to set to state to new if is not loading nor loaded (first two cases):

if ($id !== NULL)
{
    $this->load(db::select_array($this->fields())->where($this->_table_name.'.id', '=', $id));
}
elseif ($this->id) // We loaded this via mysql_result_object
{
    $this->_state = AutoModeler::STATE_LOADED;
}
else
{
    $this->_state = AutoModeler::STATE_NEW;
}

ccazette avatar Jun 30 '11 12:06 ccazette

While it is a simple change, I can also see the opportunity of that mucking up other things. For example, if someone extended __construct() and ran code before parent::__construct(), the state would be LOADING, which is wrong.

I've been aware of this "problem", but never could come up with a nice way to solve it.

zombor avatar Jun 30 '11 15:06 zombor

Mmh... Good point. I think what I suggested is closer from a correct behavior though. Another idea might solve it with an even cleaner solution anyway : why not using an additional parameter in the __constructor ? Let me explain :

public function __construct($id = NULL, $loading = FALSE) {

            parent::__construct($this->_db);

    if ($id !== NULL)
    {
        $this->load(db::select_array($this->fields())->where($this->_table_name.'.id', '=', $id));
    }
    elseif ($this->id) // We loaded this via mysql_result_object
    {
        $this->_state = $loading ? AutoModeler::STATE_LOADING : AutoModeler::STATE_LOADED;
    }
}

And then in the load method, you would simply set the loading param to true when using as_object :

 if ($limit != 1)
 {
     $query->as_object(get_class($this), array(TRUE));
 }

I haven't tested this at all, this is just an idea, but I think it could fix the issue... I just think it is pretty crucial to know if the object is loading or not when processing values in the magic __set method...

EDIT : Sorry I am not sure that will work now... Magic __set will probably be called before the status is set to loading...

ccazette avatar Jul 11 '11 08:07 ccazette