Skip to content

Instantly share code, notes, and snippets.

@everzet
Created May 21, 2012 19:45
Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save everzet/2764223 to your computer and use it in GitHub Desktop.
Save everzet/2764223 to your computer and use it in GitHub Desktop.
<?php
trait one {
public $prop;
protected function someMethod()
{
// do something hidden from the
// outside world
}
}
class two {
use one;
public $prop; // "E_STRICT: This might be incompatible..."
public function someMethod() // But this is completely OK
{
// do something publicly
}
}
// WHY ?
<?php
error_reporting(0);
trait one
{
/** prop1 */
protected $prop = 0;
/** method1 */
public function method() {}
}
class two
{
use one;
/** prop2 */
protected $prop = 0;
/** method2 */
public function method() {}
}
$two = new two();
$refl = new \ReflectionObject($two);
echo $refl->getProperty('prop')->getDocComment() . "\n";
echo $refl->getMethod('method')->getDocComment() . "\n";
#
# Will output:
#
# /** prop1 */
# /** method2 */
#
// WHY ?
@lsmith77
Copy link

The reason seems obvious to me:
There is no point in renaming properties, since the methods that use them would still break.
Renaming methods however works in many cases and therefore its supported.

As such it explains to me why property and method overloading is handled differently.

I think the RFC clearly explains this.

@everzet
Copy link
Author

everzet commented May 21, 2012

@lsmith77 I don't want to rename anything. I just want that same property with same default value and same access level wouldn't throw meaningless exception as it does now. I want behavior to be the same as it is with methods (if there's already method with same name in class - silently ignore method in trait).

@everzet
Copy link
Author

everzet commented May 21, 2012

Lemme clarify. Usecase: we have Translatable behavior trait, in which we define doctrine fields using annotations. And now we want to slightly modify field name or add relation to it by just overriding annotation, but we can't - we get meaningless E_STRICT: This might be incompatible... exception for just defining property with the same name. At the same time, "overriding" methods and changing their access modifiers is evaluated as completely OK by php. It's inconsistency at its best.

@lsmith77
Copy link

the point is that for the most part properties in traits only make sense if they are only used by the methods defined in the trait. any other method has no place in a trait definition.

as a result any property defined as part of a trait that collides with one defined in a class most likely will mean that things will blow up in strange ways. and as there is no way to handle this case explicitly, it would be dangerous to silently ignore it, because there is no language level way for you to ensure that there is no breakage.

@everzet
Copy link
Author

everzet commented May 21, 2012

@lsmith77 ok and what will happen if we'll include trait, which defines already existing method in the class, but with absolutely different functionality? It will be silently ignored, while other trait methods will be silently copied, which will 100% break things. Something that in one case is evaluated as completely harmless in other case throws exception, but i can't see any difference in both. In my opinion, it's developer responsibility to keep an eye on what methods he copies from trait and what properties this trait provides. Right now it seems ridiculous: in one case interpreter completely doesn't give a hug about collide (ignores colliding trait method) and in other tries to be smartass throwing unavoidable exception.

@lsmith77
Copy link

lsmith77 commented May 21, 2012 via email

@lsmith77
Copy link

its following standard inheritance rules in this case ..

@everzet
Copy link
Author

everzet commented May 21, 2012

@lsmith77 it's not. I can easily override property with the same signature (publics/protected) in child class.

@marijn
Copy link

marijn commented May 21, 2012

I tend to agree with @lsmith77, your annotation use-case seems weak to me (no offense). Properties from traits shouldn't be accessed by the class. Use the methods for that.

@everzet
Copy link
Author

everzet commented May 22, 2012

@marijn seems you don't understand what traits are in php (no offense). Traits are interpret-level copy-paste. There's just no way you can access properties from traits in the class, because there's simply no such thing as trait during runtime. Everything from the trait, including methods and properties is copied into final class before running your script (you can use reflection to check that). Methods are copied, properties are copied, both with their doc blocks. Every single part of the trait becomes rightful part of your class right immediately after you add use ... keyword into this class.

Problem is that in one case (methods) conflicts are ignored silently (if method's already defined in the class, even with different access level, - it's not copied from trait at all, no conflict raised) and in another (if property is already defined in the class, even with same access level and default value) - it causes E_STRICT exception. That's it. Annotations are just one example where this inconsistency becomes obvious.

@marijn
Copy link

marijn commented May 22, 2012

Ok, thanks for clearing that up @everzet. However, I still think that it doesn't make any sense to "change" the value of property (which might not have been anticipated by the trait author) or overwrite a docblock from the class that is using the trait. What would be the use case for changing a properties value? I think that problem is prevented all together when properties are initialized in the constructor, opposed to in the property declaration (though I'm fuzzy at the moment how that would work in a trait). When it comes to method "changing" in the class that uses a trait, that is just plain wrong. This might cause completely erratic behavior because the method might not be functioning anymore as intended. This would be especially harmfull when that method is used by another method defined in the trait.

It's too bad that the error handling is so inconsistent though.

@everzet
Copy link
Author

everzet commented May 22, 2012

@marijn again, i'm not talking about overriding default values or access level, i'm talking about inconsistency like that:

<?php

error_reporting(0);

trait one
{
    /** prop1 */
    protected $prop = 0;

    /** method1 */
    public function method() {}
}

class two
{
    use one;

    /** prop2 */
    protected $prop = 0;

    /** method2 */
    public function method() {}
}

$two = new two();
$refl = new \ReflectionObject($two);

echo $refl->getProperty('prop')->getDocComment() . "\n";
echo $refl->getMethod('method')->getDocComment() . "\n";

will output:

/** prop1 */
/** method2 */

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment