Hey,
1. Annotations on Annotation Classes is an overcomplication. You could
very well get rid of this feature and NOT do inheritance, leaving
it to PHP userland implementations to handle inheritance of annotations
correctly (inheritance of annotations only make sense on Class/Methods
anyways).
2. I dont get the more readable argument. The annotations themselves
would look exactly the same. They just return an array from
getAnnotation()
or getAnnotations().
You can add validation logic to any place. There is no additional benefit
of having it in the constructor. Additionally the validation if a property
exists to decide how to filter annotation arguments is not very helpful.
It can be done at any point with an array of the Annotation data also.
3.
Ok that point may be relevant, but there is also a semantically nice and
simple solution:
array('JoinTable' => array(
'name' => 'users_phonenumbers',
'joinColumns' => array(
0 => array('JoinColumn' => array('name' => 'user_id',
'referencedColumnName => 'id')),
1 => array('JoinColumn' => array('name' => 'user_id',
'referencedColumnName => 'id')),
)
));
Since an annotation is always parsed into array('Name' => array('arg1' =>
1, 'arg2' => 2)).
Annotations on the same level then get array_merge()'d.
This is why your example is an Array of JoinColumn, not an element that
has multiple join column
annotations, even in object annotations there is a difference between:
@JoinTable({...@joincolumn}) and @JoinTable(@JoinColumn)
4. Etiennes example of overwriting the constructor in a bad way is very
good. This adds
lots of overhead that can really be delegated to the userland by just
returning arrays.
5. You already mentioned further extensions with aliasing class names to
"shorten"
the annotations specification. However i see several problems with that:
a.) It adds more code
b.) Classes/Methods/Functions/Properties that have annotations of multiple
annotation libraries
will cause pain with loading or autoloading of the necessary annotation
classes.
c.) What happens if an annotation has no corresponding class?
6. A complex solution (which i think it is with objects) will lead to lots
of relevant C
code to be maintained. PHP Core C code has much longer lifecycles than PHP
code.
A lean solution using arrays only however allows different PHP based
solutions to interpret
and use them for users. These have much faster lifecycles (fixes,
extensions and such).
On Thu, 26 Aug 2010 02:26:26 -0400, Pierrick Charron
<pierr...@webstart.fr>
wrote:
> Hi Stas :
>
> I see many points to use classes instead of just arrays :
>
> 1) If you just use array, it is not possible to annotate an annotation
> to do something like [Inherited] and other eventual annotations for
> annotation.
> As Guilherme mentioned in one of his previous mail, we are thinking
> about adding a new value to the ReflectionAnnotation constructor which
> could be the reflector used to retrieve this annotation. This will
> allow developer to create other annotations for annotation like
> [Target(Target::METHOD | Target::PROPERTY)] (I'm eventually planning
> to add this Target feature in the out of the box PHP Annotation
> implementation if the concept is kept).
>
> Here is an example of what's currently implemented :
>
> [Inherited]
> class CustomAnnotation extends ReflectionAnnotation {}
>
> [CustomAnnotation]
> abstract class A {}
> class B extends A {}
>
> 2) One other advantage of using classes vs array is that it's IMHO
> more readable. Let say that you have this code :
>
> class TODO extends ReflectionAnnotation {
> const SEVERITY_CRITICAL = 1;
> const SEVERITY_IMPORTANT = 2;
> const SEVERITY_TRIVIAL = 3;
> public $value;
> public $severity = self::SEVERITY_IMPORTANT;
> }
>
> [TODO("I need to implement this class",
> severity=CustomAnnotation::SEVERITY_CRITICAL)]
> class Test {}
>
> [TODO("Review this one")]
> class Test2 {}
>
> Just by reading the code you can understand that :
> • this annotation (even if it's not used anywhere in all your
> application) can get two parameters : value, and severity,
> • the default value for severity is self::SEVERITY_IMPORTANT so you
> don't necessarily have to specify all of them
> • severity is supposed to get one of the three SEVERITY_* constant
> value (And all the constant are contained into the class itself, you
> don't have to create an extra constant somewhere else in the global
> scope or create an other class).
>
> You can also add some validation logic into the __construct if you
> want to do some validation to be sure that everything is OK.
>
> If you just see the same code without the definition of the TODO class
> you have no clue about parameters, accepted values or default values.
>
> 3) One other problem is that if you keep the annotation name as the
> array key you could have some sort of conflicts beetween annotations.
> And if you don't you loose the annotation name. Furthermore you don't
> know if something you get is an annotation in your syntax or if it is
> just a value.
>
> [JoinTable(
> name="users_phonenumbers",
> joinColumns={
> [JoinColumn(name="user_id", referencedColumnName="id")],
> [JoinColumn(name="user_id", referencedColumnName="id")]
> }
> )]
>
> Will only get one joinColumn on two if you use the annotation name as
> the key of your array (Cf just below). And when you have this array
> you don't know if name is a [Name("users_phonenumbers")] annotation or
> the actual parameter.
>
> array('JoinTable' => array(
> 'name' => 'users_phonenumbers',
> 'joinColumns' => array(
> 'joinColumn' => array('name' => 'user_id',
> 'referencedColumnName => 'id'),
> 'joinColumn' => array('name' => 'user_id',
> 'referencedColumnName => 'id')
> )
> ));
>
> If you choose to use index you'll have the following result where you
> just don't have the annotation name (even if you could add it with
> some kind of work around).
>
> array('JoinTable' => array(
> 'name' => 'users_phonenumbers',
> 'joinColumns' => array(
> 0 => array('name' => 'user_id', 'referencedColumnName => 'id'),
> 1 => array('name' => 'user_id', 'referencedColumnName => 'id')
> )
> ));
>
> Regards,
> Pierrick
>
> 2010/8/26 Stas Malyshev <smalys...@sugarcrm.com>:
>> Hi!
>>
>>> When you call getAnnotation('foo') it will just call
>>> Foo::__construct(array('value' => 'bar', 'baz' => 'hello world'));
>>> So there is no overhead on the execution time even if you have
>>> annotations in your code.
>>
>> But there's a lot of overhead if you use them - as objects are created
>> anew
>> each time - and you can't actually do anything with them (like changing
>> annotation's state) since you create them each time anew. The the
>> question
>> is why have object at all - if it can have no state? Why not just have
an
>> array?
>> I see the use case you mention in RFC is ORM mapping - which I guess
>> would
>> do just fine with arrays. Any other use cases that you see that would
>> require something more?
>> --
>> Stanislav Malyshev, Software Architect
>> SugarCRM: http://www.sugarcrm.com/
>> (408)454-6900 ext. 227
>>
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
