On Thu, 3 Jul 2025 at 23:44, Larry Garfield <la...@garfieldtech.com> wrote: > > On Thu, Jul 3, 2025, at 12:49 PM, Andreas Hennings wrote: > > >> A setter method injection is what I did in AttributeUtils, because it was > >> the only real option. > > > > In my experience, this alternative leads to more complexity in > > attribute classes. > > The constructor needs to populate the attribute with temporary values, > > which are then replaced in that separate method. > > (I call that a "setter", but only because it behaves like one from the > > outside, internally we usually don't want to store the reflector). > > > > Also, we can never be fully sure whether an attribute instance is > > "complete" or not. > > If the attribute instance is coming from > > $reflection_attribute->newInstance(), we can be confident that the > > ->injectReflector() or has been called. > > But if the attribute was instantiated manually with "new ...", it > > could as well be incomplete. > > With ReflectionAttribute::getCurrentTargetReflector(), we can fail > > early in the attribute constructor, by throwing on NULL. >
I somehow missed this reply earlier... > I ran into the "completeness problem" in AttributeUtils as well. In my case, > it's not just reflection; there's a whole bunch of other things that can be > passed back to the attribute to give it more context. There is a trade-off how much logic should live in the attribute vs the code that parses the attribute. In symfony DI there is a pattern where you register a callback for an attribute class, which would allow to make the attribute agnostic of the application or framework details. E.g. you could have the same attribute for different DI systems. (Perhaps this is not the reason why symfony does it in this way.) On the other hand, keeping logic in the attribute class also has its benefits. I did play around with dedicated interfaces for attributes per application. I am not sure I like the overly generic interfaces I see in AttributeUtils. But I would have to look deeper into it. > My eventual solution was to also have a `Finalizable` interface that is > guaranteed to be the last thing called, so the attribute can handle any > last-minute defaults. I don't love it, but given the severe limitations of > `readonly` it was the best I could do. (With aviz today, I could likely do > better.) The attribute object does not have to be your final "collectable". You can have an attribute with a method that returns something else. In fact I strongly prefer to _not_ pass attribute instances around everywhere, because they often contain clutter that becomes useless after the initial discovery. E.g. $reflection_attributes = $reflection_class->getAttributes(ServiceDefinition::class)->getAttributes(); $attribute_instance = $reflection_attributes[0]->newInstance(); $service_definitions = $attribute_instance->getServiceDefinitions(...$more_args); Or $reflection_attributes = $reflection_method->getAttributes(RouteModifier::class)->getAttributes(); $route = new Route(); foreach ($reflection_attributes as $reflection_attribute) { $reflection_attribute->newInstance()->modifyRoute($route, ...$more_args); } return $route; This way, you have a complete object in each step, but only the final object is the one you want to "collect". > > Basically, any time you have multi-step construction you will have a period > where the state is potentially incomplete. It is not just about possible incompleteness, it is about being deterministic as early as possible. E.g. if two people enter a room, it could make _some_ things much easier if you know whether they are brother and sister, or a married couple. > While passing the reflection target in is one such multi-step case, there are > lots of others, so in advanced cases (most of what I do)... When looking at existing attributes in e.g. symfony or Drupal, or my own experiments, I don't see the huge need to inject other things into the attribute. The reflector seems really the most commonly useful to me. E.g. an attribute on a method/function might have a parameter that is optional or required depending on the method's return type. Checking that in the constructor allows us to fail early, and makes everything more deterministic. > ... the object will always have an incomplete phase, and that's unavoidable. > So I'm not *that* worried about there being a brief incomplete-gap in time in > core, because I'll always have one anyway, and I've already figured out how > to handle it. I find that in most cases it can actually be avoided. But I have not really looked into your code much. I have seen some packages in packagist that use attributesutil, but I would need more time to study these. -- Andreas > > --Larry Garfield