Finding creative ways to break encapsulation isn't clever

04 Sep 2013

Don't break encapsulation

Today, I'd like to talk briefly about Encapsulaton. It's a fundamental concept in OOP. As a refresher the basic premise is:

An object should be in complete control of its state and implementation

This is why public and protected variables are discouraged and purists will tell you that any state change should be made only by a method in the object itself. It's why the topic of getters/setters is widely discussed. I don't want to get into that as it's been done to death. However, the flipside of that statement is often forgotten. Reversing this statement you can also infer that: An object should not know of the state or implementation of another object or the environment it's sitting within.

Simple, beginner OOP theory. However, I see so many advanced level developers ignoring it that it worries me. People seem to find creative ways to break encapsulation and because it's not on the list of "things not to do" like globals, digging into collaborators, public variables and static properties they think they're doing something clever.

It's not. It's not clever, it's problematic. Just because you found a way to creatively abuse language features to break encapsulation doesn't make it right. Here's a few things to add to the Things Not To Do list:

1) Annotation dependency injection

I've already spoken out against annotations but here's a practical example of the exact sort of problems they cause. Here we have a project called PHP-DI. Let's take a look at their example code:

use DI\Annotation\Inject;

class Foo {
    /**
     * @Inject
     * @var Bar
     */
    protected $bar;

    /**
     * @Inject
     */
    public function setBaz(Baz $bin) {
    }

    /**
     * @Inject({"dbHost", "dbPort"})
     */
    public function setValues($param1, $param2) {
    }
}

As you can see, the protected $bar variable is injected. Why is this a problem? Because it can only work alongside the DI framework! There is no constructor in the class and no way to provide the Bar dependency to the class without the DI Framework. Essentially, the class cannot ever work without the DI Framework. Even if that wasn't the case, and a valid constructor was provided, it's still making assumptions about the environment it's sitting within and breaking encapsulation by telling the outside world how it should be treated. Objects in OOP should care only about other objects they can directly see. In this instance, the Bar object. They shouldn't ever be concerned with things they do not have direct access to. The object is no longer in control of its own state because external parts of the application are able to alter its internal state and it tells the outside world how it should be treated.

2) ReflectionClass::newInstanceWithoutConstructor

Here's a very horrible method that can realistically only be used to break encapsulation. A constructor is provided by the class author to say "This is what the object needs in order to fulfil its responsibility". By foregoing the constructor and instantiating an object without the required dependencies or configuration you are creating an object that exists in a state in which its author did not intend. The object cannot be expected to fulfil its responsibilities and its behaviour will be unknown. This kind of "feature" in a language can only cause problems because people will find ways to abuse it. If you're using this method, you're doing something wrong and the OOP gods will smite you with fragile/buggy code.

3) ReflectionProperty::setAccessible

This is newInstanceWithoutConstructor's hyperactive little brother. You see it and you know something bad is going to happen but you're not sure exactly what or when. What this method does is overrides the class authors visibility rules and allows external code to read/write private or protected properties. Doesn't that just sound wrong already? A lot of PHP authors don't bother with private, and most use protected for the wrong reasons but the few that do are at least thinking about encapsulation and have hidden a property for a reason. setAccessible() disregards their wishes and allows external code to read/alter an object's internal state. Not only will this cause problems by potentially setting an object's state to an unexpected value and cause the object to break, but it assumes knowledge of the object's internal implementation. If the object being reflected has its internal implementation re-written (And it should be able to!) then the code reflecting it can no longer work as it was intended. All because it breaks encapsulation.

Just becuse something is possible, doens't make it a good idea. Remember the fundamental principles of OOP before rushing off to (ab)use language features. I may update this list as I discover more but I have discussed this issue in the past See: Empty interfaces are bad practice and The courier anti-pattern.