CI2 - Model class - isset() fails

[eluser]WanWizard[/eluser]
After the recent changes from _assign_libraries() to __get(), you can't use isset() or empty() anymore on any $this property.

Code:

if ( isset($this->db) && is_object($this->db) )
{
    // do something
}

always fails because isset() returns FALSE.

To fix this, the Model class needs also an __isset() magic method:

Code:

/**
* __isset
*
* Test if a CI's loaded is present
*
* @access private
*/
function __isset($key)
{
    $CI =& get_instance();
    return isset($CI->$key);
}

[eluser]Phil Sturgeon[/eluser]
Even quicker:

Code:

function __isset($key)
{
    return isset(get_instance()->$key);
}

[eluser]WanWizard[/eluser]
Quicker in execution, or in typing? :-P

[eluser]Pascal Kriete[/eluser]
I left that out on purpose :red: .

Wouldn't code like this just result in a loading call anyways? If db isn't set - load it? Then why have the if there at all, CI won't load anything twice.

I guess the problem I have with it is that the whole reason I made the change is so that CI's libraries and models wouldn't show up on the model class. With this change it feels a little like I could just assign a reference to the model in __get so it doesn't have to look it up in subsequent calls.

Convince me! :cheese:

[eluser]WanWizard[/eluser]
The reason for using isset() in this particular case is that the model needs to test if a database is present (i.e. is configured) or not.
Simply doing $this->load->database(); when the database isn't configured causes a serious error.

[eluser]Dan Horrigan[/eluser]
Actually the real code should looks something like this to avoid issues with Models have class level variables:

Code:

/**
* __isset
*
* Test if a CI's loaded is present
*
* @access private
*/
function __isset($key)
{
    if ( ! isset($this->$key))
    {
        $CI =& get_instance();
        return isset($CI->$key);
    }
    return TRUE;
}

I think there are better ways to accomplish this:

Code:

class_exists('CI_DB');

However, a lot of people are used to doing isset($this->db) and such, so it is probably worth while to throw in there. Although, if you have 2 methods using get_instance() you might as well do a class var called $ci then.


Dan

[eluser]WanWizard[/eluser]
class_exists() only works if you know the class name.

It get's complicated if you don't, or if it's variable. I prefer a general solution to this problem.

If it doesn't get into CI, no worries, that's why there's a MY_Model.

[eluser]Pascal Kriete[/eluser]
A lot of people are used to treating the model as if it were the super object, but I really want to get away from that idea.

The reason for using isset() in this particular case is that the model needs to test if a database is present (i.e. is configured) or not.
Simply doing $this->load->database(); when the database isn’t configured causes a serious error.

I still don't understand the reasoning behind doing this. Does every one of your models do this? Isn't it something you would want your application work out on installation? Or worst case - once per request?

[eluser]wiredesignz[/eluser]

Code:

$CI = CI_Controller::get_instance();

if (isset($CI->db) AND ...

Good to see _assign_libraries() deprecated.

EDIT:
The view loader _ci_load() method also assigns libraries, the CI_Loader class could also benefit from the same __get() method.

Good work Pascal.

[eluser]WanWizard[/eluser]
@pascal,

This particular model used in a module.

Modules should be agnostic to how the host application is configured, and should detect in which environment they are dropped. Modules should not make assumptions about their environment, and manual configuration isn't a good idea either.