Breaking Encapsulation with traits in PHP

Traits in PHP are dangerous because they allow the accidental breaking of encapsulation, for no real gain. In this example you can see an example of traits vs an invokable class, you could use any callable type here for the same effect. You can avoid this by being careful and not setting any member variables in traits, but everyone makes mistakes, so why risk it, when the encapsulation comes for free with callable types?

<?php

declare(strict_types=1);

use PurpleBooth\EncapsulationTraits\ExampleClass;

require_once __DIR__.'/vendor/autoload.php';

$instance = new ExampleClass();

echo sprintf("%s:\t%s\n", 'new', $instance->getName());
$instance->changeNameTrait();
echo sprintf("%s:\t%s\n", 'trait', $instance->getName());

$instance = new ExampleClass();
echo sprintf("%s:\t%s\n", 'new', $instance->getName());
$instance->changeNameInvokable();
echo sprintf("%s:\t%s\n", 'anonf', $instance->getName());

<?php

declare(strict_types=1);

namespace PurpleBooth\EncapsulationTraits;

class ExampleClass
{
    use ExampleTrait;

    private $name = 'Initial';
    private $changeNameInvokable;

    /**
     * ExampleClass constructor.
     */
    public function __construct()
    {
        $this->changeNameInvokable = new ExampleInvokable();
    }

    public function changeNameInvokable(): void
    {
        call_user_func($this->changeNameInvokable);
    }

    public function getName(): string
    {
        return $this->name;
    }
}

<?php

declare(strict_types=1);

namespace PurpleBooth\EncapsulationTraits;

trait ExampleTrait
{
    public function changeNameTrait(): void
    {
        $this->name = 'I broke encapsulation';
    }
}

<?php

declare(strict_types=1);

namespace PurpleBooth\EncapsulationTraits;

class ExampleInvokable
{
    public function __invoke(): void
    {
        $this->name = 'I do nothing';
    }
}

$ php run.php 
new:    Initial
trait:  I broke encapsulation
new:    Initial
anonf:  Initial

Code in easier to run form on github

Comments

[David Zentgraf]

While I agree that traits accessing properties is an issue in terms of guaranteed interfaces and encapsulation, I'm not sure what the callable example adds to this discussion…? It does something entirely different and seems rather unrelated to clarifying the traits issue. Can you clarify?

[Billie]

Sure, it's simply there to show there are other, more traditional, ways to implement code reuse via collaborators. These are pretty common way of reusing a single method between classes. I'm not attempting to imply that they're the only way to do it.

If I had more than 1 method to share I wouldn't have gone with the callable strategy, and rather just used a basic class.

[David Zentgraf]

Hmm, I believe a better alternative would have been what you discussed in another comment: have the trait call a method on its host class to set the name, and define that method as abstract within the trait. That establishes clear interfaces and demonstrates how to use traits "safely".

Using composition which instead does nothing doesn't seem terribly useful… :)

[Billie]

I don't agree with this.

It's too easy for people to use traits unsafely. Sure everyone on your team might know to do that, but when a new person joins your team how do you maintain that culture.

There is also a large number of people in the PHP community generally who also don't know use them safely. That's a tough cultural fight, that I'd rather not have.

Better not to have traits in the first place, use standard composition. You don't have to fight culture, and it has no negative impact on your code.

[David Zentgraf]

So basically you're saying don't use traits? That's certainly one position to have. I think they can be useful in certain situations, so eschewing them entirely is maybe throwing out the baby with the bathwater. But I agree that you need to know why they can be bad if you do use them.

[Sebastian Rapetti]

Hi

Good example, in your code the trait access a private property of the class.

When I use 'traits' I pay attention to not use class properties. It takes longer to write code that does not mix class and trait.

I think that a trait should provide only methods to a class, for extends funcionalities. Trait have own encapsulation.

[Billie]

Yeah, and you can even use abstracts to make the compiler check to see if the method is there too.

<?php

declare(strict_types=1);

namespace PurpleBooth\EncapsulationTraits;

trait ExampleTrait
{
    public function changeNameTrait(): void
    {
        $this->setName("I don't break encapsulation");
    }

    abstract public function setName(string $name): void;
}

However, I think it's still too easy for someone who doesn't know this to make this mistake. When you use other methods the encapsulation is enforced by scoping in the compiler which is much stronger, than relying on humans to not make a mistake.

[Sebastian Rapetti]

This is ok as your example with abstract:
github.com/linna/framework/blob/ma...

This need refactor:
github.com/linna/framework/blob/ma...

[Rob Waller]

That is a great example of how not to use traits. I don't think Traits are all bad though if their scope is strictly limited. They can be used as an interface for very generic functionality or to hide some of the uglier aspects of PHP.