18

Is it a code smell if the methods in my trait refer to parent::methods or to methods that are assumed to be in the utilising class?

A random (senseless) example

trait foo
{
    public function bar()
    {
        return parent::bar() . 'baz';
    }

    public function format($text)
    {
        $format = true;
        return $this->parse($text, $format);
    }
}

Are those methods to be moved in the class implementation?

Should I have exclusively methods without dependencies, inside a trait?

Kamafeather
  • 335

2 Answers2

23

It's fine if a trait depends on methods in the class into which it is embedded. But these dependencies should be explicit, by declaring an abstract function. PHP can then check that such a method actually exists.

But traits should not override methods. That is rather confusing behaviour. Instead, give a different name to the methods in the trait. In this particular example, you are relying on a method in the base class of the class into which your trait is embedded. That is a very obscure dependency. Compare the precedence examples in the docs.

Why not use abstract classes instead? Because you can have multiple traits, but only one base class. Therefore, traits are more general, and are useful for a bundle of convenience functions that you want to use in multiple classes. Your format method is a great example of this. Written more clearly, it would be:

trait Formatter
{
    abstract public function parse($text, $format);

    public function format($text)
    {
        $format = true;
        return $this->parse($text, $format);
    }
}
amon
  • 135,795
8

Yes, it's a code smell. If a trait is used to make code more portable, then then this sort of thing makes it less portable. You shouldn't do it. Without having a more concrete example it looks like what you should be doing is putting that bar method in an abstract and extending that. That approach avoids the potential runtime bug where the trait is added to a class that doesn't have bar.

zquintana
  • 254