Kirzilla
Kirzilla

Reputation: 16596

Regulating write access to object properties based on context

class SomeObject {

    protected $foo,
              $bar;

    protected $context;

    public function __construct($context) {
        $this->context = $context;
    }

    public function setFoo($val) {
        if ($this->context == 'public') {
            throw new \Exception('It is impossible to modify foo property in public context!');
        }
        $this->foo = $val;
    }

    public function setBar($val) {
        if ($this->context == 'api') {
            throw new \Exception('It is impossible to modify bar property in API context!');
        }
        $this->bar = $val;
    }

}

As you can see from this piece of "code" - object restricts setters depending on context value. This code is really hard to maintain. How can we rewrite it to make it beautiful and easy maintainable?

My thoughts are:

What's your ideas about it?

Upvotes: 0

Views: 122

Answers (4)

Cristik
Cristik

Reputation: 32870

A clean solution would be to have an ObjectFactory class that creates different objects based on a $context parameter, and having two separate classes (with a common base class) that allows writing to the appropriate properties.

Please find below a possible implementation for your schema:

/**
  * Base class that allows subclasses to define which properties are
  * writable via setters. Subclasses must not add public setters, 
  * otherwise the mechanism within this class will not work; subclasses
  * can add protected setters, though
  */
class PropertyRestricter {

    // only properties listed here are writable
    // to be initialised by subclasses
    protected $writableProperties;

    public function __construct() {
        // default implementation allows no writable properties
        $this->writableProperties = array();
    }

    public function __call($method, $arguments) {
        $matches = false;
        // check if a setter was called, extract the property name
        // there needs to be at least one argument to the setter
        if(count($arguments) && preg_match('/^set([A-Z][a-zA-Z0-9_]+)$/',$matches)) {                
            $propName = $matches[1];
            $propName[0] = strtolower($propName[0]);

            // update the property with the given value
            // or throw an exception if the property is not writable
            if(is_array($this->writableProperties) && in_array($propName, $this->writableProperties)) {
                $this->{$propName} = $arguments[0];
            } else {
                throw new Exception(get_class() . ": $propName is not writable");
            }
        } else {
            // not a setter, nor a public method
            // maybe display a user error
        }
    }
}

/**
  * Common properties for the actual classes
  */
class BaseObject extends PropertyRestricter {
    protected $foo, $bar;
}

class PublicObject extends BaseObject {
    public function __construct() {
        parent::__construct();
        $this->writableProperties = array('foo');
    }
}

class APIObject extends BaseObject {
    public function __construct() {
        parent::__construct();
        $this->writableProperties = array('bar');
    }
}

class ObjectFactory {
    public function createObject($context) {
        switch($context) {
            case 'public': return new PublicObject();
            case 'api': return new APIObject();
            default: return null;
        }
    }
}

The root of the objects is the PropertyRestricter class that allows subclasses to define which properties are writable. It makes use of the magic method __call() in order to be able to intercept setter calls and to validate the attempt to write to the property. However please note that this works only if subclasses don't add public setters for their properties.

The next level is the BaseObject class, which only defines the two properties, in order to reduce code redundancy.

The last level contains the two classes that get instantiated by the ObjectFactory: PublicObject, 'APIObject. These classes simply initialise thewritablePropertiesarray, as the rest of the work is done by thePropertyRestricter` class.

This is also a scalable solution, as it allows adding as many properties and subclasses as needed, each subclass defining its property writing rules.

Also the property update within the __call() method can be customised, I implemented it in the simplest way by directly setting the property. Actual setters can be used in subclasses and __call() can be updated to call the setters, with the mention that the setters need to be protected in order for the mechanism to work.

Upvotes: 0

deceze
deceze

Reputation: 522500

Just two general observations:

  1. You may want to segregate your classes into two parts: an immutable base class and a mutable extension:

    class Foo {
        protected $bar, $baz;
    }
    
    class MutableFoo extends Foo {
    
        public function setBar($bar) {
            $this->bar = $bar;
        }
    
        ..
    
    }
    

    This easily solves the problem when the context is defined at object instantiation time and won't ever change. Instead of instantiating with a different context which determines the mutability, you simply instantiate a mutable or immutable version of the class.

  2. If you still need more runtime checks, maybe simply using assertions is the best way to simplify the code:

    public function setBar($bar) {
        $this->assertCanSet('bar');
        $this->bar = $bar;
    }
    
    protected function assertCanSet($property) {
        if (!/* can set $property */) {
            throw new Exception("Cannot set property $property");
        }
    }
    

Upvotes: 1

Flosculus
Flosculus

Reputation: 6956

If you are trying to write good OOP, then "Interface Segregation" from the SOLID principle may be useful to you.

interface IBase
{
    public function doMethod1();

    public function doMethod2();

    public function doMethod3();
}

interface IFoo extends IBase
{
    public function setFoo($val);
}

interface IBar extends IBase
{
    public function setBar($val);
}

function doWork(IBase $obj, $val)
{
    $obj->doMethod1();
    $obj->doMethod2();
    $obj->doMethod3();

    if ($obj instanceof IFoo) {
        $obj->setFoo($val);
    }

    if ($obj instanceof IBar) {
        $obj->setBar($val);
    }
}

I doubt this example is exactly what you need, but I will use it to explain the basic idea.

A class should only have a "Single Responsibility". What that responsibility encompasses can vary however, so in general it is best to limit a class's functionality to a single area of concern as best you can.

If you want to follow "Liskov substitution", then throwing exceptions like that in your functions simply because the "context" was irrelevant, violates this principle.

Enter "Interface segregation":

By implementing an interface, you are (to a certain extent) guaranteeing to the caller of the implemented methods, that those methods will work. By excluding them, you are telling the caller that those methods don't exist.

In the example, the doWork function expects an instance of IBase, and safely calls the methods of that interface. After that, it runs introspection of the object to determine if other "applicable" methods are available.

The goal behind interface segregation is to limit the amount of unneeded features a class is forced to implement, so for you, if the context is public, it shouldn't need the setFoo method.

Upvotes: 0

Random
Random

Reputation: 3236

Maybe on the construct, fill a list of restricted methods. so, for instance :

class SomeObject {

    protected $foo,
              $bar;

    protected $context;

    protected $restrictedMethods;

    public function __construct($context) {
        $this->context = $context;
        if($this->context == 'public') {
            $this->restrictedMethods['setFoo'] = true;
        } else if ($this->context == 'api') {
            $this->restrictedMethods['setBar'] = true;
        }
    }

    public function setFoo($val) {
        if ($this->isRestricted('setFoo')) {
            throw new \Exception('It is impossible to modify foo property in '.$this->context.' context!');
        }
        $this->foo = $val;
    }

    public function setBar($val) {
        if ($this->isRestricted('setFoo')) {
            throw new \Exception('It is impossible to modify bar property in '.$this->context.' context!');
        }
        $this->bar = $val;
    }

    protected function isRestricted($methodName) {
        return array_key_exists($methodName, $this->restrictedMethods);
    }
}

Upvotes: 0

Related Questions