2
\$\begingroup\$

If I'm creating an immutable class from an existing class, should I override methods that mutate the property or have an instance of it and write my own methods to read from the property?

I have an existing class Set which is basically a fancy array. There's a few more methods in the class but for the purposes of this question they aren't relevant.

<?php

class Set implements \ArrayAccess, \Countable, \IteratorAggregate
{
    protected $items;

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

    public function has($key) {
        return array_key_exists($key, $this->items);
    }

    public function get($key) {
        return $this->has($key) ? $this->items[$key] : null;
    }

    public function set($key, $value) {
        $this->items[$key] = $value;
    }

    public function remove($key) {
        unset($this->items[$key]);
    }

    public function count() {
        return sizeof($this->items);
    }

    public function offsetExists($offset) { return $this->has($offset); }
    public function offsetGet($offset) { return $this->get($offset); }
    public function offsetSet($offset, $value) { $this->set($offset, $value); }
    public function offsetUnset($offset) { $this->remove($offset); }

    public function getIterator() {
        return new \ArrayIterator($this->items);
    }
}

Now I want to create ImmutableSet and here are two options that I came up with:

class ImmutableSet extends Set
{
    public function set($key, $value) {
        throw new \Exception('Cannot set things');
    }

    public function remove($key) {
        throw new \Exception('Cannot remove things');
    }
}

The problem I see with this approach is the existence of set and remove for an immutable object doesn't seem right and supposing that the Set class implemented a method for example clear then I'd have to override that too.

Alternatively, I can do this:

class ImmutableSet implements \Countable, \IteratorAggregate
{
    private $items;

    public function __construct(array $items) {
        $this->items = new Set($items);
    }

    public function has($key) {
        return $items->has($key);
    }

    // Implement get, count, and getIterator to call $items->
}

My concern with this design is basically the opposite of the other one. Supposing that Set implemented a method getAllKeys, I'd have to implement it here too to benefit from the functionality.

What is the better approach here?

\$\endgroup\$
2
\$\begingroup\$

You are right it seems rather weird for an immutable object to provide setters, which will throw an exception no matter how you call it. I would use inheritance and two different interfaces in this situation.

Imagine the base set. I would argue the base class should be the immutable set, since the immutable set declares functionality which both classes should be able to use. An example could be:

interface ImmutableInterface {

    public function fetch($key);
    public function exists($key);

}

class ImmutableSet implements ImmutableInterface {

    protected $items = [];

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

    public function fetch($key)
    {
        return $this->items[$key];
    }

    public function exists($key)
    {
        return array_key_exists($key, $this->items);
    }

}

You now have a basic implementation. You make your set manipulatable you could write a new interface declaring methods for changing values inside your set. The new interface could be:

interface ChangeableInterface {

    public function add($key, $value);

}

You could then write a sub-class of your original immutable set and implement the new interface. Since the $items array was declared protected any sub-class also has access to it.

class ChangeableSet extends ImmutableSet implements ChangeableInterface {

    public function add($key, $value)
    {
        $this->items[$key] = $value;
    }

}

You can now type-hint against a specific required implementation by using the interfaces. You also have the opportunity to use other classes as long as they implement the specific interface.

You can think of this strategy as declaring the fewest amount of required methods for your class to work. If you want to add more functionality you write an interface declaring no more than the required methods for that functionality to work. You can even take this further. Imagine not all your changeable sets should be able to remove items again. This could be an interface:

interface RemoveableInterface {

    public function remove($key);

}

You would then write a new sub-class of either the ChangeableSet or ImmutableSet, depending on your requirements, and implement the new interface. You can now type-hint specifically against sets which are only able to remove items. This strategy can continue forever, but do be careful. This can also be too much. Too many sub-classes can become a hell to maintain. So try to keep the amount of interfaces low, while still maintaining your required flexibility and the method signatures they declare concise.

The interface/class names used in these examples are not set in stone. They are by purpose very descriptive, so you can always rename them. Also keep in mind there are no error checking in the examples.

I hope this can help you, happy coding!

\$\endgroup\$
  • 2
    \$\begingroup\$ I thought it might be useful to mention that (as far as i can tell) "declaring the fewest amount of required methods for your class to work" is another way of stating the "interface-segregation principle" which is the I in SOLID. en.wikipedia.org/wiki/Interface_segregation_principle \$\endgroup\$ – Victory Aug 16 '15 at 16:50
  • 1
    \$\begingroup\$ @Victory - You are right. Couldn't remember the appropriate name when i wrote this. \$\endgroup\$ – AnotherGuy Aug 16 '15 at 19:06
  • 1
    \$\begingroup\$ I would argue the base class should be the immutable set, since the immutable set declares functionality which both classes should be able to use - no, it doesn't. Immutability is not a "functionality", it's a contract. As I wrote in another comment, mutability is contradiction of immutability, not its extension. How can any object be immutable and mutable at the same time? That's why immutable classes should be final, that's why in strong-typed languages (like Java or C#) String is final. \$\endgroup\$ – Konrad Morawski Aug 17 '15 at 10:40
  • \$\begingroup\$ @KonradMorawski - You are right about immutability being a contract. But I would argue since this isn't Java and the base class stays immutable, this may be one solution for this issue. \$\endgroup\$ – AnotherGuy Aug 17 '15 at 17:08
  • \$\begingroup\$ @AnotherGuy I think this is something language agnostic, it's just how OOP is supposed to work. Why would PHP be different than Java here? \$\endgroup\$ – Konrad Morawski Aug 17 '15 at 19:35
-1
\$\begingroup\$

I would recommend doing it the other way around, making ImmutableSet the base class and encapsulating the functionality you need.

class ImmutableSet implements \ArrayAccess, \Countable, \IteratorAggregate
{
    protected $items;

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

    public function has($key) {
        return array_key_exists($key, $this->items);
    }

    public function get($key) {
        return $this->has($key) ? $this->items[$key] : null;
    }

    public function count() {
        return sizeof($this->items);
    }

    public function offsetExists($offset) { return $this->has($offset); }
    public function offsetGet($offset) { return $this->get($offset); }
    public function offsetSet($offset, $value) { $this->set($offset, $value); }
    public function offsetUnset($offset) { $this->remove($offset); }

    public function getIterator() {
        return new \ArrayIterator($this->items);
    }
}

You then inherit and implement the extra methods:

class Set extends ImmutableSet
{
    public function set($key, $value) {
        $this->items[$key] = $value;
    }

    public function remove($key) {
        unset($this->items[$key]);
    }
}

This would happen if we were to follow your naming convention, however that could also change. You should call the base class Set and the child class MutableSet, which seems to make more sense in terms of inheritance.

The pattern is not complete if you can't change from one to the other, however, which requires implementing copy methods and constructors, such as copy and immutableCopy. You'll notice that this is the exact pattern used by Objective-C frameworks.

Edit for further clarification:

What I mean is that you need to implement a mechanism of turning mutable sets into immutable ones and vice-versa. Take the following example:

function foo(Set $set)
{
    $set = (MutableSet)$set;
    $set->set('key', 'value');
}

// Somewhere
$set = new MutableSet();
foo($set);

Disclaimer: the code isn't tested and I haven't written anything in PHP for a while, there might be issues, even syntactic.

Notice how the set is assumed to be immutable but it still mutates, and it's perfectly valid code. This depends on how far you want to take this, because if it's a requirement to prevent situations like these you'll need to create copies:

$set = new MutableSet();
foo($set->immutableCopy()); // Or just "copy"

This would prevent situations like the one I described. Again, this is the pattern used by Objective-C, I'm not making anything up.

\$\endgroup\$
  • \$\begingroup\$ Can you elaborate on what you mean by "The pattern is not complete if you can't change from one to the other, however, which requires implementing copy methods and constructors"? \$\endgroup\$ – rink.attendant.6 Aug 16 '15 at 19:13
  • \$\begingroup\$ I ended up using a similar approach by creating abstract class Set, class MutableSet extends Set and class ImmutableSet extends Set. \$\endgroup\$ – rink.attendant.6 Aug 17 '15 at 6:58
  • \$\begingroup\$ Please see my edit. \$\endgroup\$ – André Fratelli Aug 17 '15 at 9:40
  • \$\begingroup\$ Set extends ImmutableSet - sorry, but it's extremely poor design. Your mutable Set would return true for instanceof ImmutableSet, which is a blatant lie. It's misuse of inheritance. Mutability is a contradiction of immutability, not its extension. Immutability promises something, and your subtype breaks that promise. @rink.attendant.6's alternative where MutableSet and ImmutableSet are siblings is much better. If you named the base class ReadableSet instead of ImmutableSet, that would be much better - since mutability entails both writing and reading, no contract is broken. \$\endgroup\$ – Konrad Morawski Aug 17 '15 at 10:26
  • \$\begingroup\$ @KonradMorawski that's why I proposed a name change and instead made Set the base class and MutableSet the child class. Don't get mislead by the name, a Set is a container whether you are capable of adding objects to it or not, so instanceof Set would be appropriate, whether it is mutable or immutable. It's written in the answer. This is the pattern used, not a pattern, on situations like these. Like I said, I'm not making anything up. See here. \$\endgroup\$ – André Fratelli Aug 18 '15 at 5:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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