PHP Traits: If they weren’t evil, they’d be funny

When traits first saw the light of day, with the release of PHP 5.4, I didn’t really think much of it. I believed (and still do) that the alleged shortcomings of single inheritance exist mainly in the mind of the developer. If ever you run into the situation where you say to yourself “Damned, if only there was a way to inherit from both these classes”, chances are there’s a flaw with either one of the existing classes, or the one you’re writing.
The most commonly, the root cause of the problem is code that doesn’t adhere to the SOLID principles. If you’re not too familiar with concepts like DI, SRP, LSP and such, you might want to read up on them first, because I’m assuming a reasonable understanding of these principles throughout this post.

Anyway, back to multiple inheritance, or rather: back away from it. I’m not the only one to say that multiple inheritance is something to avoid as much as possible, and people Bjarne Stroustrup (creator of C++) has said that there’s nothing you can do with multiple inheritance that can’t be done with single inheritance. The full quote/reference can be found in the conclusion of this rant.

But that’s something I could drone on about for eons: the reasons why I believe traits are almost completely pointless don’t really matter. I do see a few valid use-cases for them, which I’ll discuss at the end of this post, but none of them require anything like the complex “features” PHP has bestowed on the trait.

And therein lies the problem. A trait is just that: a characteristic. A trait is not something that defines the way a person works, or what he/she is capable of doing: having a lower voice doesn’t change anything about the way you eat. It’s just a trait that you may or may not share with any number of people who are, in terms of functionality and definitive properties, exactly the same as you.

Let’s look at how traits might fit in when attempting to represent the real world in OO PHP code:

abstract class Creature
{
    public $alive = true;//set to false in destructor?
    public $name = null;
}

class SingleCellOrganism extends Creature
{}

abstract class Vertebrate extends Creature
{
    public $gender = null;
    public $hotBlooded = false;//primitive = default
}

Interface Mammal
{
    public function reproduce(Creature $with);
    public function breathe();
    public function lactate();
}

class Human extends Vertebrate
    implements Mammal
{}

Now ignoring the omissions and inaccuracies, both in terms of knowledge of the animal kingdom and the suggested hierarchy itself. In a structure like this, as it is in the real world, all humans can exhibit “traits”, like: being an optimist, called Steve. A proponent of traits would then write something like:

trait Optimist
{
    public $smile = true;//default state is smiling
    public function givePepTalk(Creature $to = null)
    {}
    public function seeTheBrightSide(Situation $story)
    {}
    //other aspects of this rather irritating trait
}

class Steve extends Human
{
    use Optimist;
    public $gender = 'male'
}

At first glance, all seems to be well. If ever Steve, the eternal Optimist, finds another Creature, preferably of the Vertebrate-extending mammal-implementing Human class, he might end up passing down this Optimist trait. Joy unbridled, we have a trait we can actually reuse. So what’s my problem?

For starters: traits in PHP are non-communicative. By that I mean: they’re not easy to detect. If Optimist were an interface, or an other level of parent classes (which it should’ve been, because it adds functionality), we would be able to write code like this

    public function debbyDownerNeedsPep(Optimist $creature)
    {
        //explain situation
        $creature->seeTheBrightSide($this->getMyStory());
        //now tell me how those ugly flowers that give me allergies are really nice
        $creature->givePepTalk($this);
    }

This code is, no matter how you look at it, the best way to go: you type-hint for a particular interface or base class (ie a contract, a given, known unit of functionality), and you use it. The type-hint communicates to the user that this method is aware of the Optimist contract, and knows how to use it. That’s all you need to know to use this method.

But traits can’t be type hinted for, so instead we have to write more code to detect the constructs we need, and use because they “reduce code-duplication”:

    public function negativeNancyNeedsPep($someObject)
    {//in theory, Optimist can be used by any object, so the Creature type-hint might be too restrictive
        if (!is_object($someObject)) {
            throw new InvalidArgumentException(
                sprintf(
                    '%s expects argument to be an object, %s given',
                    __METHOD__,
                    gettype($someObject)
                )
            );
        }
        $usedTraits = class_uses($someObject);
        if (!isset($usedTraits['Optimist'])) {
            throw new InvalidArgumentException(
                sprintf(
                    '%s requires object that is an Optimist',
                    __METHOD__
                )
            );
        }
        //do stuff
    }

Now, the reason why some people are enthusiastic about traits is because they cut-down on code duplication. This implies that the code that ends up being shoved into a trait is used all over the place, in various classes which are being used all over the place. In scenario’s with even slightly more complicated inheritance levels (like in the example above: Steve has 3 parent levels and an interface, probably implemented by one the parents already), there’s a good chance you’ll be writing a lot of code that is almost exactly the same as the next piece, except for the name of the trait you’re looking for.
Now you may be thinking I’m exaggerating here, and maybe I am. I hope that no sane person would ever dream to hope to even think of possibly considering to rely on traits to such an extent that code like this becomes a requirement. However, what I am dead-serious about is that the checks that I perform here are NOT over-the-top. What’s more, as I’ll demonstrate in the next bit: they’re still too lax and very unreliable!

It gets Worse!

So far, I’ve been assuming that no other object will implement the methods or properties that are defined in the trait, nor have I considered the possibility of name conflicts and breaches of contract! This is where things get really nasty.

The first mistake I’ve made was when I wanted to make sure the Optimist methods existed in that negativeNancyNeedsPep method. I assumed that if the trait wasn’t used by the instance passed as an argument, I couldn’t continue. However, as the class_uses documentation states: its return value doesn’t include classes used by the parent class! So my checks are incomplete. Replacing them with a method_exist call will fix this issue, but it could result in a false positive: other objects could implement a method with the same name, but those methods can do entirely different things. Because I’m relying on traits, I can’t use a targeted type-hint, so I’m left with a choice: use method_exists and hope for the best, or use a complex, slow and frankly silly piece of code to test for Traits:

function objectHasTrait($obj, $trait)
{
    //check arguments
    $used = class_uses($obj);
    if (!isset($used[$trait])) {
        $parents = class_parents($obj);
        while (!isset($used[$trait]) && $parents) {
            //get trait used by parents
            $used = class_uses(array_pop($parents));
        }
    }
    return isset($used[$trait]);//return bool
}

That’s a nice example of an ingenious solution to a problem that never should have existed in the first place. Or at least: that’s how I feel about this kind of code. If you have to inspect the argument you receive, to see how it is constructed (from a code point of view), you’re doing something wrong. If you buy a car, and need to know how its engine was built, just to be able to steer the damn thing, people would look at you funny, and probably wonder what you’re doing outside of the insane asylum.

Note: this is just the tip of the iceberg. Next, I’ll move on to the really serious issues with traits. The reason why they’re tragic, hilarious, dangerous and far too complex.

Don’t mess with my contract

Whenever a class implements an interface, or extends another class, the child class inherits the parent’s interface as a contract. That is to say: if a parent declares a method to be public, the child must offer the same method with at least equal access. If a parent declares a protected method, then the child must do the same. It can’t make the method private, but it can make it public.

Just as extending a class is meant to be used to add functionality an existing unit of code, so too should its contract be treated: keep, or reduce the restrictions, never tighten them.

Overriding methods is allowed, obviously, but again, the signature of the parent must be followed. Additional arguments should be optional, existing type requirements (type-hints) can be reduced to some extent, but can’t be strengthened:

protected function parentMethod(array $foo, Human $bar)
{}

//valid child version – extends contract, without breaking it
public function parentMethod(array $foo, Creature $bar, DateTime $addThis = null)
{}

//illegal child version – breaks contract on 3 counts
private function parentMethod(array $foo, Steve $bar, DateTime $added)
{}

Because traits can contain abstract methods, properties as well as method implementations, they form an integral part of the object’s contract, or so you might think. In reality, though, nothing could be further from the truth:

trait Oops
{
    final public function iAmFinal()
    {}
}

class Not
{
    use Oops;
    final protected function iAmFinal()
    {}
}

This will NOT give an error, the protected method will be used and declared as final. The final keyword is, essentially, meaningless when used in a trait. Which means traits will never be able to enforce a contract as strictly as a class can. So why am I bothered by this? Because even though traits can’t really ensure a contract (I’ll list some more reasons why they can’t), they do boast the capability of enforcing part of its users’ interface through abstract method declaration:

trait OopsNoMore
{
    abstract public function iAmFinal();
}

This doesn’t make ANY difference. If the class that uses the trait defines a method with the same name, the trait’s declaration is simply ignored. It’s not in a position to enforce any contract of any sort. It can’t even decide on what the visibility or names will be of the methods it implements:

trait WhyBother
{
    protected function iAmNotPublic()
    {
        echo 'This is a protected method, called iAmNotPublic';
    }
}

class YesYouAre
{
    use WhyBother { iAmNotPublic as public nowYouAre; }
}
$x = new YesYouAre;
$x->nowYouAre();//echoes This is a protected method called iAmNotPublic

So what have I demonstrated with this? That whatever visibility you define in the trait is easily overruled by the class that uses it, and that whatever name you give the method is completely arbitrary, too. You can even alias a trait method to an old-school PHP4 constructor (which will issue a notice and is therefore evil).
If you though I was being funny where I said that the code showing what checks were required to compensate for not being able to type-hint traits wasn’t enough, consider this: I know that trait X is being used, but I have no guarantee whatsoever that the names of the method have remained the same!

All these things actually make traits like this hazardous:

trait MoreMethods
{
    public function dependsOn(SomeType $x)
    {
        $y = $this->otherTraitMethod($x);
    }
    private function otherTraitMethod(SomeType $x)
    {
        //do stuff
    }
}

If the public method is invoked, it requires the otherTraitMethod to be defined in a specific way. As I’ve shown: conflicting definitions in the using class, or usage of an alias might result in weird errors: the method required might have changed name, it might have a different signature, it might even come from a different trait! That’s right: we’ve been operating under the assumption that the trait was included as a whole, which needn’t be the case at all:

class CombineTraits
{
    use MoreMethods, EvenMore { EvenMore::otherTraitMethod insteadof MoreMethods; }
}

Now if you still felt like traits were a great feature, I hope that by now you’re feeling a bit bewildered and perhaps slightly ill at ease, hold on to that feeling, it means your gut-feeling is telling you there is something wrong here.

There’s something wrong, but it’s not the idea of traits

Let me be clear: I am not, by any means opposed to traits in certain (albeit rare) cases. They really CAN be a tool for reducing code duplication. But that’s all they’re good for. They shouldn’t be allowed to declare abstract methods, you shouldn’t be able to pick-and choose what methods you use, how and even less so: what to call them. A trait should be a take it or leave it kind of thing that just does a very simple, basic, and rather common task.

Here’s a simple, valid use-case for traits that can be used in data models:

trait EmailTrait
{//include Trait in the name, to avoid accidental Old-school constructor usage
    protected $email = null;
    public function getEmail()
    {
        return $this->email;
    }
    public function setEmail($email)
    {
        if (!filter_var($email, FILTER_VALIDATE_EMAIL)) {
            throw new InvalidArgumentException('Invalid email');
        }
        $this->email = $email;
        return $this;
    }
}

In an ideal world, traits like this should be accompanied by an Interface, to compensate for the lack in type-hints, to enforce a public contract, and to avoid aliases from removing certain method names.
In case you are wondering: Do I use traits like this? Nope, I don’t. If you wonder why I don’t here’s a couple of reasons why:

  • The implementation of traits is bad, as I’ve attempted to explain in this post.
  •  Most IDE’s will automatically generate getters and setters. Sure, there’s duplicate code, but I didn’t write all of it. Writing a trait like the one above probably took longer than having all of these duplicate getters/setters generated in the first place.
  •  I genuinely haven’t felt the need for them.
  •  I’ve never gone as far as to micro-optimize to the point of comparing duplicate code to traits, but I suspect traits are actually going to slow the code down (autoloading, more IO and I suspect the trait implementation is based on the way PHP implements abstract classes internally, so a trait might translate to about the same overhead as an additional inheritance layer)

To best sum it up, here’s a truly terrifying example of how evil traits can be (note: this code works, that’s what should scare you):

trait EvilTrait
{
    /**
     * @var int
     */
    public $foobarCalls = 0;
    /**
     * final AND protected, even if you override it, you shouldn't be allowed to make it private, right?
     * @return string
     */
    final protected function getSomeString()
    {
        return 'This was returned by a protected method called getSomeString';
    }
    /**
     * Abstracts are useful to ensure certain methods exist, and have a particular signature
     */
    abstract protected function someAbstractMethod();

    /**
     * Be careful with method names like this, if the using class has the same name, it might become
     * an old-school constructor... Oh, did you know that PHP4-style constructors emit E_DEPRECATED notices?
     */
     public function foobar()
     {
         $this->foobarCalls++;
     }
}
class Foobar
{
    //using an alias AND changing visibility, hmmm... that may not be a good idea
    use EvilTrait { getSomeString as public iSeeYou; }

    /**
     * Abstract declares this method as protected, so only protected and public
     * should be allowed, this is private, I'd expect a fatal error here:
     */
    private function someAbstractMethod()
    {
        return 'This should not be allowed';
    }

    /**
     * We've implemented an abstract method in an illegal fashion
     * Let's check if we can call this contract-breaking implementation:
     */
    public function testAbstractImplementation()
    {
        return $this->someAbstractMethod();
    }

    /**
     * Note: using as <alias> doesn't mean the original method-name is forgotten
     * It only means that you can call it by a different name, AND that its visibility
     * depends on the alias... $x->getSomeString() doesn't work
     */
    public function demonstrateAlias()
    {
        return $this->getSomeString();
    }
}

$x = new Foobar();
var_dump($x->foobarCalls);
echo $x->testAbstractImplementation(), PHP_EOL,
    $x->iSeeYou(), PHP_EOL,
    $x->demonstrateAlias();

Sure enough, run this code and this is the output you’ll get:

int(1)
This should not be allowed
This was returned by a protected method called getSomeString
This was returned by a protected method called getSomeString

So calling $x-&gt;iSeeYou() really does invoke the protected getSomeString method… I can’t even tell you how insane this is. Is it likely that you’ll stumble on code that uses traits like this? Of course not! No person, sane or otherwise, would be capable to maintain code like this, writing unit tests for this kind of code is even more insane. How do you test the visibility of aliases? How do you determine what method is invoked when and where? You really can’t, especially if you start mixing traits together, and use the insteadof keyword.

Conclusion:

Traits are, all in all, a missed opportunity IMO. They could’ve been a sensible feature to reduce code duplication, if they’d kept the number of “features” down. But they didn’t, they tried to implement traits as an optional, partial, abstract contract so what you end up with is a feature that is neither here nor there. In true PHP style, the way you use it is your own responsibility or, if you’re going trait crazy, your own problem.

There are a number of blog-posts out there telling you how fantastic traits are, and what you can use them for. I’ve yet to find a single one that actually warns about the pitfalls that they bring to the party. Seeing as PHP is a language that is very accessible, there are a lot of people out there writing a lot of code that isn’t really representative of what PHP can do in the right hands. I fear that traits, as they are implemented at the moment, are going to be hugely popular with people who might not be fully aware of the very sensible, simple, but vital SOLID principles.
There is a real chance for traits to become the next eval: the first resort people will use to solve issues that should’ve never have existed in the first place.

If you’ve read through this gibberish, and if you were to take anything from this rant, let it be this:

Use traits if you want to, but use them to reduce code duplication only. Don’t try to enforce contracts with them: they’re just not capable. Steer well clear of the more complex “features” like the aliasing and the insteadof keyword. And do your absolute best to avoid using multiple traits. Traits are weak contracts, to avoid the diamond problem, usign multiple weak contracts reintroduces that same diamond problem, which is what insteadof and as are meant to solve, but really: they’re moving the problem instead of fixing it. Ironically, 9/10 times, if you really look at your code, using a trait is merely moving the problem, too: if you write your code well (following what I believe to be the most important of the SOLID principles: Single responsability), single inheritance really is enough.

Don’t take my word for it, many people who are well versed in C++ (which does support multiple inheritance) will tell you multiple inheritance is not needed, and generally a bad idea. Even the person who created C++ (Bjarne Stroustrup) said as much:

“People quite correctly say that you don’t need multiple inheritance, because anything you can do with multiple inheritance you can also do with single inheritance. You just use the delegation trick I mentioned.”

Read the full text here

An alternative to traits?

Is there an alternative – Yes, of course there is. As the quote above states: you can use the delegation trick (using DI). Let’s take the Optimist trait example we used earlier, and see how we can implement that functionality without traits:

//first, create a base for traits, an abstract class

abstract class Trait
{
    protected $name = '';

    protected $users = [];

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

    final public function registerUser($usedBy)
    {
        $this->users[] = $usedBy;//this is not the optimal way, use an assoc array if possible, but still...
        return $this;
    }

    final public function deregister($user)
    {
        //here's why $this->users should be an assoc array
        foreach ($this->users as $k => $obj)
        {
            if ($obj === $user)
            {//remove circular reference
                unset($this->users[$k]);
                break;
            }
        }
        return $this;
    }
}

class OptimistTrait extends Trait
{
    protected $name = 'Optimist';//use constants for this
    //same methods as above go here, except for one crucial difference:
    //pass user instance to this method
    //additional params depend on the method
    public function setSmile($user)
    {
        //don't use $this here, use $user
    }
}
class Creature
{
    protected $traits = [];//default is no specific traits

    /**
     * Allow passing of traits via the constructor, if you want to...
     */
    public function __construct(array $traits = [])
    {
        foreach ($traits as $trait)
            $this->addTrait($trait);
    }

    final public function addTrait(Trait $trait)
    {
        $this->traits[$trait->getName()] = $trait->registerUser($this);
    }

    /**
     * @return bool
     */
    final public function hasTrait($name)
    {
        return isset($this->traits[$name]);
    }

    public function dropTrait($name)
    {
        if (isset($this->traits[$name]))
        {
            $this->traits[$name]->deregister($this);
            unset($this->traits[$name]);
        }
    }

    public function __destruct()
    {
        foreach ($this->traits as $trait)
        {
            $trait->deregister($this);
        }
        $this->traits = null;//make sure the instances are no longer referenced
    }
}
class Steve extends Human
{
    public function __construct(array $traits = [])
    {
        //set default traits in constructor
        //you could declare a $requiredTraits property that prevents
        //the dropTrait method from removing these
        $traits[] = new OptimistTrait;
        parent::__construct($traits);
    }

    /**
     * Expose default/required traits directly
     */
    public function doOptimistThings(Creature $with)
    {
        $this->tratis['Optimist']->callMethod($this, with);
    }

    /**
     * If you want to, though I'd not recommend this, you can expose the traits directly
     * Note that __call is slow, and this makes your code more error prone and harder to test
     * @throws BadMethodCallException
     */
    public function __call($method, array $args)
    {
        foreach ($this->traits as $trait)
        {
            if (method_exists($trait, $method))
            {
                array_unshift($args, $this);//prepend instance to args
                return call_user_func_array([$trait, $method], $args);
            }
        }
        throw new BadMethodCallExcpetion(
            sprintf(
                '%s::%s is not callable, trait is missing or method is not defined',
                __CLASS__,
                $method
            )
        );
    }
}

Now we can easily check whether or not a particular instance has a particular trait using a simple $instance-&gt;hasTrait('Optimist'); call. Furthermore: if some traits are required, we can simply declare the trait methods in the class itself. And before you go on to say that this defeats the point, and causes code duplication, let me point out that: yes, it does cause some code duplication: a method declaration with a single line: return $this-&gt;traits[$name]-&gt;method($this, $arguments);. Thats one line, not the same method that is repeated throughout. It also allows you to add something extra to the trait method: execute it and perhaps tally the number of calls on that trait (for whatever reason). Using this approach, these “traits” act more like parent methods, so it’s actually a more accurate way to mimic multiple inheritance:

public function inheritExample()
{
    $this->calls[__METHOD__] += 1;
    return parent::inheritExample();
}
vs
public function pseudoTraitExample()
{
    $this->calls[__METHOD__] += 1;
    return $this->trait['name']->pseudoTraitExample();
}

Another objection you might have with this approach is that it is seemingly more verbose. But is it really? remember: there’s no need for tons of checks to see whether or not an object actually implements a given trait. You’ll probably write this code once, and basically use that abstract trait-supporting class as base for all of the classes that will require traits. I really don’t think that there will be that much of a difference in terms of lines of code, or duplicate lines at all.

Even if there were a difference, I still find this (using separate classes instead of weak abstract subsets) the better option. Calling hasTrait tells you all you need to know, you just know what methods will be available, and what they’ll do. There’s no aliases to muck things up, and no diamond problem to worry about. The traits act as a sort of namespace, so there’s no chance of method-name conflicts, whereas combining traits in classes leaves the conflict resolution up to the implementing class, which means it’s something that can change depending on who resolved the conflict, or what method was needed by the last person working on the code.

Advertisements
This entry was posted in opinion, programming, Uncategorized, webdevelopment and tagged . Bookmark the permalink.

25 Responses to PHP Traits: If they weren’t evil, they’d be funny

  1. Pingback: Traits , nice solution to the nasty mess of inheritance | kingvish

  2. Mauro says:

    Great post.

  3. Steve says:

    If I have a class Animal and a class Car, and I want to use geolocation on them, is it one of the good idea to use traits ?

    I don’t like traits in general, but I feel they’re useful in these cases where you don’t want to extend with no sense (Animal extends GPS, Car extends GPS ?), and both classes will share the same methods. Am I right ?

    • eliasvo says:

      I understand the reasoning behind using traits in such a particular scenario. I’ve been thinking on posting a more extensive follow-up, focussing on _Valid_ use-cases for traits, too. However, in this particular case, the usage of traits might be regarded as a breach of the SRP. An `Animal` instance represents a living creature, not its location. An `Animal` class _Can_, however, have a location set (through a setter), by passing it to a `GeoLocator` service, which in turn can use any dependency to locate/track cars, animals, … Basically, like in real life, a GPS is something that is _added_ to a car, or injected (as in a chip) into an animal’s skin. The locating mechanism is a separate entity, with its own contract and dependencies. You can inject it into any class you want to.

      What I’m basically saying in a round about way is: geolocation in your scenario is something I’d implement using services and DI, not traits.

      • bartdorsey says:

        It’s the name that’s wrong.. a Trait like this would be called “Locatable” and yes, both Cars and Animals can be locatable by GPS.

  4. David Graham says:

    What about using Delegation to avoid inheritance. You can use traits to take away the repetitiveness of this approach. For example, bring in the dependency “Optimist” to YourClass. Then add a trait “OptimistTrait” that has all the methods of Optimist that you want available for clients of YourClass. The trait includes each method and its delegation to Optimist’s method. Of course you would need to update YourClass interface/contract to match these new methods offered from the trait. Later, if you decide to add more things to Optimist, you can update YourClass interface and trait to reflect those added features. But you don’t have offer those features, it’s your choice.

    • eliasvo says:

      What you’re basically suggesting, then, is to maintain a contract (ie the methods in the Optimist dependency) in 2 places (the class itself, AND the trait). That rather defeats the point of traits as being a tool to reduce code duplication. At the end of this post, I set about giving a crude and really basic example of using injection + delegation, so I’m with you when you say delegation is the way if you want to write “composition-over-inheritance” style code.

      Like I said in last comment, if I find the time, I’ll post a follow-up piece on more valid use-cases for traits, and some more disadvantages they bring (especially in terms of writing testable code). The idea of having delegation methods in place is quite close to something I want to highlight in the follow-up post: traits are a valid, and in some cases even a valuable, refactoring tool: copy the old methods to a trait, and set about overriding them in each class that is using that code separately, so you can refactor at your own pace without breaking inheritance chains… I really need to find time to post a follow-up I think 🙂

      • David Graham says:

        I’d say it should be a requirement that someone properly understand polymorphism, composition, inheritance and SOLID principles effectively before touching Traits. So to a new developer they can be used for evil, just like inheritance, but they aren’t inherently evil. But your post does express the inherent danger!

        I think we are on the same page here about how to use Traits correctly (for example they help avoid writing the boilerplate code, ie. decorators, to meet the interface implemented by our class). As far as saving you typing, I don’t see the point defeated here. Traits have nothing to do with saving you work when one of your dependencies altered their contract. It’s your choice to alter your own contract to match, or just adjust your internal code to work with the changed dependency.

        I’d say I have found two primary uses for traits (very useful IMO):
        #1. Create my own “BarClass” and separate out code from that class into a “use DecoratesFooTrait” so all the boilerplate code is separated away from the interesting code (easier on the eyes for myself and other devs). Imagine if I were to offer 20 different various decorators (ie. text renderer, pdf renderer, html renderer, etch-a-sketch render, etc…) then I can just add this trait to each, and just focus on the code of each that adds the new/adjusted functionality.
        #2. Move code that I would normally have in a parent class into a Trait and put it in the child class, removing the need for extending the parent. I call this a “DominantTrait”. It’s a tightly coupled trait (no different from inheritance) and my strict rule is I only have one of these in a class. I do this for situations where I have classes that require some logic to be provided by a child class (parent typically has abstract methods) that I can’t work around elegantly with just composition. I may even go so far as to have a __construct in the trait as well (*crowd gasps*, I know, and I can give you my rationale for this if you want). All other traits (like the decorator mentioned in #1) are subordinate to this trait. The main goal is to free up the “extends” so folks can use my class/package and add their own “base class” (for things like site-wide configuration, etc.)

        Too many traits is of course is an SRP code smell. Too many traits could also cause some potential for naming collisions (and confusion from renaming). This would be equivalent to the nightmare of multiple and deep inheritance. Thanks for posting and also talking with me. I’m just now getting into traits and came across your blog. It’s nice to see folks on a similar page. 🙂

  5. Bas says:

    Great article, I always seem to end up here whenever I get the bad idea to use a trait 🙂 I generally agree with you, traits are largely redundant and promote bad code in the hands of many coders. However, I don’t quite agree with your interpretation of what should happen in the EvilTrait example.

    I don’t think you should treat traits like they are part of a contract or like they are an OO structure. They are not. They are macros and should be used to paste repetitive, stand-alone code.

    In the EvilTrait example you seem surprised that you are allowed to ‘implement’ an abstract method defined in a trait by a function with a different visibility level. And that you are able to ‘override’ a final method. But a trait is not a base class and cannot enforce a contract to a class using the trait. There is no ‘implement’ and ‘override’ when using a trait, those are properties that belong to classes and interfaces.

    What I am surprised about is that we are allowed to redeclare (!) an existing function name. After all, we’ve pasted an abstract method into a class and then declare another function with the same name. In my opinion that is the counter intuitive part, that everything declared in a trait is silently overwritten. They could have at least provided some sort of warning or notice. (preferably error)

    • eliasvo says:

      I agree that we shouldn’t view traits as part of a contract. They’re clearly a compiler-assisted copy-paste. My problem with this is that traits allow you to use keywords like final and abstract, but can’t really enforce a contract. I’m not as anti-trait as this post may have you believe, but I am against traits as they are implemented. If you can change the visibility of a trait method, then you shouldn’t be told (in the PHP docs) that you can ensure a method is present by declaring an abstract method.

      A trait can be useful if you use it like an implemented interface, that you copy-paste (eg use ImplementationTrait) into an existing class. Requiring the using class to write an implementation for abstract protected function X(array $arg) whilst at the same time accepting that implementation to look like private function X(DateTime $arg). Take a look at this example of how traits can be used to break otherwise perfectly valid code. Again: I’m not saying that it’s likely to come across code like that, but it’s possible. If a feature allows you to write something like that, I think that feature is badly implemented.

      I’ve said it a couple of times: I’m thinking about writing a follow-up to this post, showing how I believe traits can be used properly. What parts of the idea I actually like, and how they can make our lives easier. This rant was mainly me complaining about a possibly useful feature being implemented badly, and therefore potentially opening the door to bad practices and bad code. Something that has plagued PHP for far too long, IMO (something I mentioned in my Sympathy for the Devil post).

      So in resuming: Yes, I agree with you that traits aren’t all bad. If you see them as macro’s (or copy-paste’s), then you’re absolutely right to use a trait if you find yourself re-using the same methods in several classes. I’m just trying to warn people who use traits as a way to actually inherit from more than one true parent (ie inheriting from multiple contracts). If you do that, then you’re solving a problem that shouldn’t have existed in the first place.

      • Bas says:

        Oh, I wasn’t trying to argue whether or not traits are all bad. I agree with you there.

        In your example in the link I have an issue with your comments in the Foobar class.

        1. Above the ‘someAbstractMethod’ declaration you say that you shouldn’t be allowed to set it to private, because you are modifying the access level. This is an incorrect way of looking at it and is only valid if the abstract class would be declared inside a base class. The Foobar class does not ‘implement’ the abstract method of the trait. You’ve simply pasted the code of the trait into the class and it should be treated as a double declaration of a method. Not an improper implementation.

        2. The same goes for the ‘getSomeString’ method. You’re not ‘overriding’ the method and making it public. You are redeclaring it.

        This seems like nitpicking but it’s not. It is a fundamental difference between viewing a trait as an OO construct and a macro. Point 1 above should (imo) throw the error “Fatal error: Cannot redeclare Foobar::someAbstractMethod()”. I get the impression that you would expect it to throw “Fatal error: Access level to Foobar::someAbstractMethod() must be protected or weaker”.

      • eliasvo says:

        I think we’re both saying the same thing, more or less. The reason why I’m saying changing visibility shouldn’t be allowed is because the PHP documentation states that

        “Traits support the use of abstract methods in order to impose requirements upon the exhibiting class.”

        Whereas the Liskov Substitution Principle explicitly forbids a child class to restrict visibility of inherited properties/methods. Traits are said to allow you to enforce a partial contract, but they clearly allow you to break the LSP. Either you can enforce a partial contract (and therefore cannot reduce visibility), or you can’t enforce a contract. If you can’t enforce a contract, a trait should either not be allowed to declare abstract methods, unless you’re only using it in abstract classes exclusively, or you shouldn’t be allowed to override trait methods (and instead an error should be triggered).

        You seem to favour the error being triggered, I’m sort of in the middle. Either way, the current trait implementation is either inconsistent (a copy-paste that only pastes methods that aren’t implemented already), or lacking (enforcing a contract, whilst allowing the user to break it easily).
        And once again: I really feel like I should take the time to write a follow up, maybe exploring both approaches (the one you suggest: error re-declaring method X) vs access level fatal errors a bit more. Before I do that, though, I’ll have to look into the trait implementation in a bit more detail, and see if some things have changed for PHP7

      • Bas says:

        Indeed we do agree and I didn’t see that part in the documentation. That is just very weird and completely counter intuitive. It is not how I think a trait should work and judging by your examples that is not how it works in practice either.

        If that is how they were really meant to work (and I highly doubt it) the implementation is completely broken. On the other hand if it was meant as a glorified copy/paste mechanism (and I think it was) that part should be removed from the documentation. I looks like an error in the docs by someone who didn’t completely understand how traits work.

        The current implementation seems to be a macro (copy/paste). If there is anything in a class that duplicates what is in the trait, PHP will silently override the contents of the trait with that of the class.

      • eliasvo says:

        Yup, completely agree. The implementation is the problem. The fact that the documentation gives elaborate examples of things a trait “could be used for” is, IMO, only causing people to wrongly assume that declaring abstracts is the right thing to do. I don’t know where I read it, but one of the PHP core contributors nikic (Nikita Popov) described traits as a compiler assisted copy-paste. No more, no less. I think it’s safe to say that that’s how we must see them, and we must use them as such. To try and do anything more with traits would be wrong.

        Until the actual implementation is fixed (which is, I fear) not a likely thing to happen any time soon, I hope this post does enough to prevent some people from using traits in the wrong way. Either way: thanks for reading this rant, hope you enjoyed it, and thanks for the feedback 🙂

  6. phpdev says:

    Why is there this apparent obsession with class member visibility? Honestly, this feature seems like it causes far more problems than it’s worth. The number of times I have to extend a class (in a library or something) simply so that I can create a public getter for a protected/private property or method is ridiculous. I don’t see a single advantage to this. I understand that for a library, you want to maintain a consistent public interface for other developers to use, in case you refactor things in the future. Fine. But in your own code? Just seems like it’s creating more work for you.

    • eliasvo says:

      Visibility is an important part of OOP. I’d even argue it’s more important in PHP than other (strong typed) languages. You can prevent users from assigning random values to private/protected members, or at least validate the values through use of getters and setters.

      I’ve never found myself in the situation where I had to extend a library class. If a library did not allow me to do what I needed to to, I’d look for an alternative. However, it’s up to you to make sure you didn’t miss anything (ie other ways to set whatever it is you need to set).

      I don’t see how you can run into problems with non-public members in your own code though. Surely that means you made a mistake earlier on in the design, wouldn’t you say?

  7. ak47 says:

    Another property of trait composition is that the composition order is irrelevant, and hence conflicting trait methods must be explicitly disambiguated (cf. section 3.5). Conflicts between methods defined in classes and methods defined by incorporated traits are resolved using the following two precedence rules.

    – Class methods take precedence over trait methods.

    – Trait methods take precedence over superclass methods. This follows from the flattening property, which states that trait methods behave as if they were defined in the class itself.

    You can read more here: http://scg.unibe.ch/archive/papers/Scha03aTraits.pdf

    http://stackoverflow.com/questions/11521403/php-5-4-why-can-classes-override-trait-methods-with-a-different-signature

    I think traits make sence if you know how to use them. Traits are extensions, so the idea is to extend with extra functionalities and not really to mess overriding methods. Also traits generally should have an interface for being implemented in the class that uses them so its informed about the extension.

    • eliasvo says:

      Yes, class methods take precedence, but in doing so, the signature is completely ignored. If a trait allows you to define an abstract with a given method, but the class implements it differently (ie abstract is public, the class implements a private method), that’s bad. At the very, very least I expect a warning.

      I have no issues with traits as such. Again: as almost anything: if you know what you’re doing, and understand the caveats, you can use anything to your advantage. The way traits were described/promoted when they were added was as if they were the most useful thing ever. Which they are not. The aim of this post was to point out the dangers and problems that traits pose if used as a magic bullet.

  8. P0lT10n says:

    Before all, I know this article is “old”.

    I read all the article, really interesting (I know nearly nothing about traits) but I got your point, and learned something about Traits…

    I can’t believe there are some articles in other blogs that says “Traits are the best things that could happened to PHP”… I googled “PHP Trait” and found lot of articles about that… I really can’t believe it… People that argue that “Traits” are really good, better than classes or interfaces or even inheriting, they know zero about SOLID, OOP and OOD.

    It drives me crazy think that some people comments that traits are still good to use… Imagine this in C/C++/C#, it’s an abomination to OOP and OOD. And I must say that I have now 4 years of experience with PHP and nearly 2 with C#… I am not a Senior but c’mon people… Books (ebooks if you like) don’t bite…

    Thank you really much on writing this article, It gave me hope that there are still people that knows to code with PHP… I see a lot of “StackOverflow” and “Github repos” that are so baddly developed that I was thinking PHP was for kids…

    • P0lT10n says:

      I forgot to add that most of “Traits” are like “Helper” classes, right ?

      • eliasvo says:

        Nikic (one of the core contributers to PHP, responsible for some great changes like PHP7’s AST) describes them as compiler-assisted copy-pastes. Nothing more, nothing less. That actually added to my outrage: If all a trait did was copy-paste code in, then I could live with that. It’s the whole idea of adding class-like features (eg abstract, final, the insteadof keyword to address the diamond problem, …) that make them behave like helper classes. And of course, if all you need is a helper class: Inject a helper class. That’s why I included the quote at the start.

        I think traits started out as a simple automated copy-paste (almost an include), with some additional checks. Gradually features were added that give them this confused status, where a trait can include abstract definitions, but not enforce them correctly.

        An analogy would be: starting to build a guarden shed, but gradually adding things like plumbing, central heating and the like. In the end, you end up with something that has many features one might find in a house, but the result is not up to code: it’s unsafe, and very likely to break in the near future.

        Anyway, PHP has many messy sides, and ropey features. People who are familiar with the concepts of good code, and know the many caveats and problems that exist within PHP can build great things. It’s just a shame that not enough people question whether or not things are in fact valuable assets (eg PHP7’s AST, scalar types, and vastly improved memory management), or just the next hot-ticket item/ coding hype (traits, or anonymous functions, which in PHP are actually instances of the non-exposed Closure class).

    • eliasvo says:

      Thanks for the reply. Indeed it’s been a while since I wrote this rant. I get what you’re saying about PHP being an ecosystem plagued with horrible repo’s, full of code written by people who don’t know the first thing about the core principles of OOP and such.

      Since writing this, I have in fact moved from PHP development to writing Go/Golang most of my time. I had several reasons for doing so, but I’d be lying if I said the quality of some of the code out there wasn’t a factor.

      Anyway, thanks for the feedback again, and spread the word!

  9. cap says:

    There is an easy way to check if something has a trait. Well, indirectly. Whenever I write a trait I pair an interface along with it.

    interface OptimistInterface {
    public function smile();
    }

    trait Optimist {
    public function smile() { … }
    }

    class Steve implements OptimistInterface {
    use Optimist;
    }

    Now just have to check if Steve is an OptimistInterface.

    • eliasvo says:

      Yes, pairing with an interface is more communicative. I’d argue that it’s good practice. That wasn’t the point I was trying to make. This rant focuses mainly how traits are documented/presented. They were “announced” as a way to work around the alleged limitations of single inheritance (IMO, there is no such thing). More over, they decided to bombard traits with pointless, and badly broken features (eg the use of abstract in traits), and the possibility of manual conflict resolution, breaking the contract. I’ve said this a few times, but I’m now working on a follow-up of sorts. I’ve switched from PHP to Go, and I’ll be posting a rant on composition over inheritance, and the misconceptions surrounding it.

  10. This post is proven truth. Period!

    The only thing so far i found traits helpful was:

    – injecting high level short hand functions, pointing to SOLID code (like symlinks)
    – so i could kind of ‘overload’ the argument list (and do some magic before calling the ‘real’ methods)

    You may think of it as an additional – and in every aspect of the word optional – api layer 😉

    Keep up posting & discussing – this is critical truth ❤

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s