3
\$\begingroup\$

I created this class to keep track of a user's role:

class UserRoleHelper extends AppHelper
{
    public $helpers = array('Session');

    private $role = NULL;

    const CUSTOMER  = 'customer';
    const ASSOCIATE = 'associate';
    const MANAGER   = 'manager';
    const ADMIN     = 'admin';
    const DEVELOPER = 'developer';

    private $score = array(
        self::CUSTOMER  => 0,
        self::ASSOCIATE => 1,
        self::MANAGER   => 2,
        self::ADMIN     => 3,
        self::DEVELOPER => 4,
    );

    public function role()
    {
        if($this->role == NULL)
        {
            $this->role = strtolower($this->Session->read('Auth.User.role')) ?: 'customer';
        }
        return $this->role;
    }

    private function is($role)
    {
       return $this->score[$this->role()] >= $this->score[$role]; 
    }

    public function admin()
    {
        return $this->is(self::ADMIN);
    }

    public function manager()
    {
        return $this->is(self::MANAGER);
    }

    public function associate()
    {
        return $this->is(self::ASSOCIATE);
    }

}

The idea is to be able to have an expression like:

if($this->UserRole->manager())
{
    //do something
}

And have it evaluate true if you are a manager or higher (admin, developer).

I also tried to design it so if there is ever the need to add a level like GENERAL_MANGER = 3 which would shift the values of ADMIN and DEVELOPER up one, it won't effect anything.

I originally had the keys in the $score array as strings, but it seemed redundant when I had constants already defined with the same values. I have never seen an array definition with constants for keys in a class body before. I know variables are not allowed but it seems to work with constants. Still don't know if that is bad or not though.

Is there anything I have missed or can do to make this better?

\$\endgroup\$
0

2 Answers 2

2
\$\begingroup\$

Use an enumerator?

And use it like this:

if($this->isInRole(Roles::MANAGER)
\$\endgroup\$
1
0
\$\begingroup\$

It looks fine, the internal logic is encapsulated.

Maybe this number comparison based role logic will not work if you have VIP_CUSTOMER and VIP_MANAGER roles and VIP_MANAGER's can't manage non-VIP CUSTOMERs/functions and simple MANAGER's can't manager VIP_CUSTOMERs/functions. In this case you can change implementation without any change in the client code to support this.

(For example the VIP_MANAGER's score is 3 and MANAGER's score remains 2 then every VIP_MANAGER would be a MANAGER. If MANAGER's score is 3 and VIP_MANAGER's score is 2 then every MANAGER would be a VIP_MANAGER. None of them is good but you can modify the manager (and the imaginary vipManager) method to support this. )

Changing the internal logic also a nice solution when the MANAGERs are not allowed to buy anything, so they shouldn't have the CUSTOMER role. You just change the manager method and it will work.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.