Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm building a symfony project and at some point I've come up with a switch case to manage the acl rights. I would prefer using a dynamic access to the constant but havn't find a good solution. I've seen reflection but that doesn't seem to be the proper solution to me.

$mask = null;

switch ($participant->getRight()) {
    case 'VIEW':
        $mask = MaskBuilder::MASK_VIEW;
        break;
    case 'EDIT':
        $mask = MaskBuilder::MASK_EDIT;
        break;
    case 'OPERATOR':
        $mask = MaskBuilder::MASK_OPERATOR;
        break;
    default:
        break;
}
$this->aclManager->addObjectPermission($project, $mask, $user);

So, this switch is in a foreach, that gets the $user entity, then with the result of the list box from the view, I get the rights of the user which can be (for now) EDIT VIEW or OPERATOR. These right were chosen because they are directly related to the MaskBuilder masks.

The thing is, if there is a refactor to be done, I will need to modify this switch AND the part where the form is defined. I would like to do something like

$mask = 'MASK_' . $participant->getRight();
$this->aclManager->addObjectPermissions($project, MaskBuilder::$mask, $user);

But havn't find a way to do it.

Is my switch a good way to do it ? Or is there a proper php way to do the same, that would reduce refactor cost?

share|improve this question

4 Answers 4

up vote 3 down vote accepted

I found the solution somewhere from the documentation. I don't know why I had not seen that before.

Symfony2 provides an easier way of doing what I was looking for:

$builder = new MaskBuilder();
$mask = $builder->add($participant->getRight())->get();
share|improve this answer

If you want to keep the constant names intact, you could use:

$mapping = array(
    'VIEW' => MaskBuilder::MASK_VIEW,
    'EDIT' => MaskBuilder::MASK_EDIT,
    'OPERATOR' => MaskBuilder::MASK_OPERATOR,
);

$mask = array_key_exists($participant->getRight(), $mapping) 
        ? $mapping[$participant->getRight()] 
        : null;
$this->aclManager->addObjectPermission($project, $mask, $user);

Or alternatively, put the mapping function on your MaskBuilder (especially if you don't have outside access to the constants on MaskBuilder):

class MaskBuilder
{
    // (...)
    public static function getMask($right)
    {
        $mapping = array(
            'VIEW' => self::MASK_VIEW,
            'EDIT' => self::MASK_EDIT,
            'OPERATOR' => self::MASK_OPERATOR,
        );

        if (array_key_exists($right, $mapping)) {
            return $mapping[$right];
        }

        return null;
    }
}

// (...later...)
$this->aclManager->addObjectPermission(
    $project, 
    MaskBuilder::getMask($participant->getRight()), 
    $user
);
share|improve this answer
    
Yeah I dont have access to map builder, it's a built-in librairy in Symfony –  Hugo Dozois Jan 31 '13 at 14:11
    
@HugoDozois You might consider creating your own class that contains the 'getMask()' method. I.m.o. this answer/solution is clean and flexible (I update the answer above to reflect this option) –  thaJeztah Feb 1 '13 at 19:43

You mean something like this?

$mask = constant('Symfony\Component\Security\Acl\Permission\MaskBuilder::MASK_' . $participant->getRight()) ?: null;
$this->aclManager->addObjectPermission($project, $mask, $user);

Or if you don't care about readability:

$this->aclManager->addObjectPermission($project, constant('Symfony\Component\Security\Acl\Permission\MaskBuilder::MASK_' . $participant->getRight())?:null, $user);
share|improve this answer
    
I'll definately try it tomorrow! I had try something similar with constant but it wasn't working. Something about private member, but maybe this way will work –  Hugo Dozois Jan 30 '13 at 22:43
    
Couldn't find constant MaskBuilder::MASK_VIEW Since the constant is defined in another file I don't have access to it directly with the constant function. –  Hugo Dozois Jan 31 '13 at 14:14
    
That sucks :( I'm all out of ideas on that front then (aside from require_once()'ing the file that contains the MaskBuilder class... –  jsanc623 Jan 31 '13 at 15:43
    
Yeah I thought about that but that would be breaking the way symfony include files because I already have a use .../MaskBuidler statement –  Hugo Dozois Jan 31 '13 at 16:09
1  
constant works fine, you just have to use the fully qualified classname: constant('Symfony\Component\Security\Acl\Permission\MaskBuilder::MASK_VIEW') –  Peter Feb 24 '13 at 19:36

I would create a final type code class:

<?php

final class Mask {

    private $_name;
    private $_value;

    private static $_view;
    private static $_edit;
    private static $_operator;

    public static function View() {
        if (self::$_view == NULL) {
            self::$_view = new Mask("View", 1);
        }
              return self::$_view;
    }

    public static function Edit() {
        if (self::$_edit == NULL) {
            self::$_edit = new Mask("Edit", 2);
        }
              return self::$_edit;
    }

    public static function Operator() {
        if (self::$_operator == NULL) {
            self::$_operator = new Mask("Operator", 4);
        }

              return self::$_operator;
    }

    private function __construct($name, $value) {
        $this->_name = $name;
        $this->_value = $value;
    }

    public function Name() {
        //can be localized or create a new method to get the localized name
        return $this->_name;
    }

    public function Value() {
        return $this->_value;
    }

    public function __toString() {
        return $this->_value;
    }

}
  • Type hinted usage
  • Can be add more method to the class

Helpful thing can be to create a static method to get all type, and another one to parse the type from a value. Whith these things a $participant->getRight() could return an instance of Mask so the switch can be removed.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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