Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

State problem #46

Open
ccazette opened this issue Jun 30, 2011 · 2 comments
Open

State problem #46

ccazette opened this issue Jun 30, 2011 · 2 comments

Comments

@ccazette
Copy link

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;
}
@zombor
Copy link
Owner

zombor commented Jun 30, 2011

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.

@ccazette
Copy link
Author

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 ccazette reopened this Jul 11, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants