2016-10-24 9:42 GMT+02:00 Michael Morris <tendo...@gmail.com>:

> On Mon, Oct 24, 2016 at 3:21 AM, Michał Brzuchalski <
> mic...@brzuchalski.com>
> wrote:
>
> >
> >
> > 2016-10-24 8:45 GMT+02:00 Michael Morris <tendo...@gmail.com>:
> >
> >> On Sun, Oct 23, 2016 at 3:39 AM, Michał Brzuchalski <
> >> mic...@brzuchalski.com>
> >> wrote:
> >>
> >>
> >> https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Comp
> >> onent%21Assertion%21Inspector.php/function/Inspector%3A%3Aas
> >> sertAllObjects/8.2.x
> >>
> >
> > You're missing is_object on $member in foreach in your implementation
> >
>
> Wrong - read the code more carefully. The function is variadic. If only 1
> argument is passed then is_object is checked against all members of the
> array to test.  If 2 or more arguments are passed than the instanceof
> operator is used instead. This operator will return false when testing non
> objects.
>
>
> > and it won't be even needed if there where object type hint.
> >
> >
>
> Wrong again. The method tests a collection (array or traversable) to see if
> each member satisfies its test.  Type hints do not work that way at all.
> Even if they did, this code must work under PHP 5.5.
>
> If you're going to criticize someone's code at least read over it
> carefully. And it wouldn't hurt to check the unit tests either since they
> do exist for this method. See here:
>

You're right I didn't read it carefully. I was in my way to work and little
bit hurry.
also it is true that my patch doesn't affect your use case in any way
you've pointed out.


>
> https://api.drupal.org/api/drupal/core%21tests%21Drupal%
> 21Tests%21Component%21Assertion%21InspectorTest.
> php/function/InspectorTest%3A%3AtestAssertAllObjects/8.2.x
>
> Best practice is to test for interfaces or more rarely classes. This RFC
> encourages people to not do this so I'm not in favor of its inclusion.
>



-- 
regards / pozdrawiam,
--
Michał Brzuchalski
about.me/brzuchal
brzuchalski.com

Reply via email to