On Thu, Apr 16, 2020, at 1:46 AM, Benjamin Eberlei wrote:

> > > > 3. I see the most common case for attributes being getting the object
> > > > version.  With the reflection API as currently described, I see two
> > > > shortcomings.
> > > >
> > > > A) I can't tell if an attribute has a valid object or not before
> > trying to
> > > > access it, which would presumably fail spectacularly.  I believe we
> > need a
> > > > way to know if getObject() is going to return a valid value before
> > trying
> > > > to call it.  I think this is a hard-requirement.
> > > >
> > > > B) Related, as is getting all attributes as objects looks to be rather
> > > > clunky.
> > > >
> > > > $attribute_objectgs =
> > array_filter(array_map(function(ReflectionAttribute
> > > > $r) {
> > > >   if ($r->getObject()) { // Needs something better here.
> > > >     return $r->getObject();
> > > >   }
> > > > }, $obj->getAttributes()));
> > > >
> > > > That's gross. :-)  Can "get all the attributes that can be formed into
> > > > objects" be its own operation?  $obj->getAttributeObjects() or some
> > such,
> > > > that skips over non-instantiable attributes and instantiates the rest?
> > > >
> > >
> > > I don't see A.) what would you do when the object instantiation fails?
> > You
> > > would throw an exception I presume, let the engine throw the regular
> > > TypeError, ArgumentError, Error if class not exists that everyone is
> > > already familiar with.
> > >
> > > For B.) I believe you are extrapolating based on your own use case.
> > Working
> > > with Reflection is usually a lot of boilerplate, I don't believe we need
> > to
> > > have a one liner here.
> >
> > It depends on the annotation, I suppose.  If I'm requesting a specific
> > annotation by name, presumably I know if it is supposed to have an
> > associated class.  If it's supposed to but it's missing, that's a legit
> > class-not-found exception/error.
> >
> > However, I'm thinking of cases where code is integrating with a 3rd party
> > optionally, through an annotation.  In that case it's a fair question of
> > whether the class will be defined or not based on whether some other
> > library is present.
> >
> > Similarly, if a bit of code is requesting all attributes (as above) rather
> > than just specific ones by name, it wouldn't know if a given attribute is
> > supposed to be defined or not; as written, class-less attributes are
> > supported.
> >
> > I suppose the workaround would be class_exists($r->getName()).  Weird but
> > I guess works?  It would have to be documented as a thing you should do,
> > though, which implies to me that it could be made cleaner.
> >
> > That reflection is usually clunky today (true) is to me not a compelling
> > argument that it shouldn't be made less clunky. :-)
> >
> 
> You are not safe from these problems when using Doctrine Annotations either
> (missing library or class does not exist) and it fails exactly the same way
> as trying to instantiate something that doesn't exist.
> 
> I also realized why IS_INSTANCEOF is not the default, because it needs to
> resolve all attributes to classes to perform the check. This triggers
> autoloading *all* attributes of the reflected declaration (even the ones
> not requested), so we felt it should not be the default.

Ah, valid.  I suppose that's an unavoidable result of allowing non-class-mapped 
attributes, which means anyone building on it is stuck doing their own 
class_exists() check for everything.

Sad panda.  (Still very +1 on the RFC, just sad panda about these details.)

--Larry Garfield

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to