$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