$this->propertyValues['is_active'] shouldn't that be rather an overridable method?

Georg Gell <[email protected]> Mon, 29 Aug 2005 19:55:27 +0200
Newsgroups gmane.comp.php.pear.liveuser
Message-ID <[email protected]>
hello list,

i am still playing with the propel container.
I found this code in common.php::login():

        // If login is successful (user data has been read)
        // ...we still need to check if this user is declared active
        if (!array_key_exists('is_active', $this->propertyValues)
            || $this->propertyValues['is_active']
        ) {

I think that this is not good style, because
$this->propertyValues['is_active'] is not set in common.php, but in a
child class. I think that this should be in a method that can be
overridden by the child class,

so the above code should look like:

        // If login is successful (user data has been read)
        // ...we still need to check if this user is declared active
        if ($this->isActive() {

and this method should be added to class common:

	function isActive(){
		return (!array_key_exists('is_active', $this->propertyValues)
            || $this->propertyValues['is_active']);
	}

because then I could use the common::login() method and override
common::isActive() with something like this:

	function isActive(){
		return $this->user->isActive();
	}

without passing return values through undocumented properties. What do
you think?

Ok, another point:
in LiveUser::readConfig() the defaults for the cookies are set.
Shouldn't this be in globals.php?

And a last thing:
in LiveUser::fileExists(), the function file_exists is used. But this
function checks the uid of the file, so when you are running
safe_mode=on, the containers will not be found, even if you could
include them. is_readable() can be used to prevent to prevent this.

Regards
Georg