Monday, 2 July 2018

PHP: Expectation management with Symfony Validator validation

G'day:
I was checking out of of our web services the other day, replicating a bug I was fixing, and found the need to guess at what the body content needed to be for a POST I was doing. Our validation is solid, so I could just start with an empty object, and the validation would tell me what I needed to pass. Or, yes, I could RTFM but I couldn't be arsed.

Anyhow, this tactic worked for the first few properties: the validation came back with "you need this property"... "nah, it needs to be an integer"... "that's too low", etc. Cool. But then I passed an empty string for one of the properties, and instead of getting a 400 back with a hint as to what the constraints needed to be; I instead got a 500 server error. F***'s sake.

{
    "errors": [
        "Internal Server Error"
    ]
}

This is good and bad. Bad it was a 500 for some reason; good that there's no detail bleeding out. This was part of what I was fixing.

Looking at the logs I was getting this:

{"errors": ["Expected argument of type \"array or Traversable and ArrayAccess\", \"string\" given"]}

Indeed I was passing in a string:

{
  "firstName" : "Zachary",
  "lastName" : "Cameron Lynch",
  "address" : ""
}

(not the actual data in question, but it's work code so I cannot share it). Right so address is supposed to be an object, eg:

{
    "firstName" : "Zachary",
    "lastName" : "Cameron Lynch",
    "address" : {
        "street" : "123 Some Street",
        "city" : "Our City"
    }
}

That sort of thing.

But anyhow, why's the valiation failing? Well not failing - I'd expect / want that - but why is it erroring? I assumed we were doing something wrong: not validating it was an object, and just charging off and using it. I looked into it, and our code seemed legit, so start trying to make a solid repro case so I can ask people out in the world about it.

I know nothing about Symfony's Validator component. I have code-reviewed other people using it, so am vaguely aware of how it works, but I've not really used it. First things first then... the docs.

I concocted this test case from the introductory docs:

use PHPUnit\Framework\TestCase;
use Symfony\Component\Validator\Constraints\Length;
use Symfony\Component\Validator\Constraints\NotBlank;
use Symfony\Component\Validator\Validation;

class StringValidatorTest extends TestCase
{
    public function testExampleFromDocumentation()
    {
        $validator = Validation::createValidator();
        $violations = $validator->validate(
            'Bernhard',
            [
                new Length(['min' => 10]),
                new NotBlank(),
            ]
        );

        $this->assertCount(1, $violations);
        $this->assertEquals(
            'This value is too short. It should have 10 characters or more.',
            $violations[0]->getMessage()
        );
    }
}

And this worked as expected:

PHPUnit 7.2.6 by Sebastian Bergmann and contributors.

.                                                                   1 / 1 (100%)

Time: 218 ms, Memory: 4.00MB

OK (1 test, 2 assertions)

Also this is pretty straight fwd. Create a validator, pass a value and some constraints to a validate, and... job done.

From there I needed to test the behaviour of passing the wrong data type when expected. I decided expecting an integer and then not giving it one would be the easiest test here:

First a class to wrap-up the validator with the constraints in question:

namespace me\adamcameron\general\validation\collectionIssue;

use Symfony\Component\Validator\Constraints\Type;
use Symfony\Component\Validator\Validation;

class IntegerValidator
{

    private $validator;
    private $constraints;

    public function __construct()
    {
        $this->validator = Validation::createValidator();
        $this->setConstraints();
    }

    private function setConstraints()
    {
        $this->constraints = [
            new Type(['type' => 'integer'])
        ];
    }

    public function validate($value)
    {
        return $this->validator->validate($value, $this->constraints);
    }
}

Nothing surprising there. I have a constraints collection which is simply "gotta be an integer". And a method that takes a value and validates it against those constraints.

And then some tests:

<?php

namespace me\adamcameron\general\test\validation\collectionIssue;

use me\adamcameron\general\validation\collectionIssue\IntegerValidator;
use PHPUnit\Framework\TestCase;

class IntegerValidatorTest extends TestCase
{
    use ViolationAssertions;

    private $validator;

    protected function setUp()
    {
        $this->validator = new IntegerValidator();
    }

    /** @dataProvider provideCasesForValidateTests */
    public function testValidate($validValue)
    {
        $violations = $this->validator->validate($validValue);
        $this->assertHasNoViolations($violations);
    }

    public function provideCasesForValidateTests()
    {
        return [
            'has value' => ['value' => 42],
            'is null' => ['value' => null]
        ];
    }

    /** @dataProvider provideCasesForViolationsTests */
    public function testValidateReturnsViolationsWhenConstraintsBroken($invalidValue)
    {
        $violations = $this->validator->validate($invalidValue);
        $this->assertHasViolations($violations);
    }

    public function provideCasesForViolationsTests()
    {
        return [
            'string' => ['value' => 'forty-two'],
            'integer string' => ['value' => '42'],
            'float' => ['value' => 4.2],
            'integer as float' => ['value' => 42.0],
            'array' => ['value' => [42]]
        ];
    }
}

So I have two tests: valid values; invalid values. I expect no violations from the valid values, and some violations from the invalid values (I don't care what they are, just that there's a violation when the value ain't legit):

PHPUnit 7.2.6 by Sebastian Bergmann and contributors.

.......                                                             7 / 7 (100%)

Time: 204 ms, Memory: 4.00MB

OK (7 tests, 14 assertions)

(Oh, I whipped a coupla custom assertions out into a trait):

namespace me\adamcameron\general\test\validation\collectionIssue;

use Symfony\Component\Validator\ConstraintViolationList;

trait ViolationAssertions
{
    public function assertHasNoViolations($violations)
    {
        $this->assertInstanceOf(ConstraintViolationList::class, $violations);
        $this->assertEmpty($violations);
    }

    public function assertHasViolations($violations)
    {
        $this->assertInstanceOf(ConstraintViolationList::class, $violations);
        $this->assertNotEmpty($violations);
    }
}


Bottom line, as you can see from the test results, one can pass an invalid value to a validator, and it won't error; it'll just return you yer violation. Cool.

So what's the go with a collection then? Why are we getting that error?

I concocted a similar CollectionValidator:

namespace me\adamcameron\general\validation\collectionIssue;

use Symfony\Component\Validator\Constraints\Collection;
use Symfony\Component\Validator\Constraints\Type;
use Symfony\Component\Validator\Validation;

class CollectionValidator
{

    private $validator;
    private $constraints;

    public function __construct()
    {
        $this->validator = Validation::createValidator();
        $this->setConstraints();
    }

    private function setConstraints()
    {
        $this->constraints = [
            new Collection([
                'fields' => [
                    'field1' => [new Type(['type'=>'integer'])]
                ],
                'allowMissingFields' => true,
                'allowExtraFields' => true
            ])
        ];
    }

    public function validate($value = null)
    {
        return $this->validator->validate($value, $this->constraints);
    }
}

This time I'm defining a "collection" (read: "array" in this case) which could have a property field1, and if it does; it needs to be an integer. It could have other properties too: don't care. Or indeed field1 could be missing: don't care. But if it's there... it needs to be an integer.

And we test this. I'll break the class down this time so I can discuss:

namespace me\adamcameron\general\test\validation\collectionIssue;

use me\adamcameron\general\validation\collectionIssue\CollectionValidator;
use PHPUnit\Framework\TestCase;

class CollectionValidatorTest extends TestCase
{
    use ViolationAssertions;

    private $validator;

    protected function setUp()
    {
        $this->validator = new CollectionValidator();
    }

    /** @dataProvider provideCasesForValidateEmptyCollectionTests */
    public function testValidateWithValidButEmptyCollectionTypeIsOk($validValue)
    {
        $violations = $this->validator->validate($validValue);
        $this->assertHasNoViolations($violations);
    }

    public function provideCasesForValidateEmptyCollectionTests()
    {
        return [
            'array' => ['value' => []],
            'iterator' => ['value' => new \ArrayIterator()]
        ];
    }
}


First just a baseline test with some empty (but valid) values. These pass:

PHPUnit 7.2.6 by Sebastian Bergmann and contributors.

..                                                                  2 / 2 (100%)

Time: 187 ms, Memory: 4.00MB

OK (2 tests, 4 assertions)


Next I test a fully-legit collection with a valid integer value for the field1 value.

public function testValidateWithIntegerField1ValueShouldPass()
{
    $violations = $this->validator->validate(['field1'=>42]);
    $this->assertHasNoViolations($violations);
}

All good:

PHPUnit 7.2.6 by Sebastian Bergmann and contributors.

.                                                                   1 / 1 (100%)

Time: 211 ms, Memory: 4.00MB

OK (1 test, 2 assertions)

And now with the correct field1 property, but without an integer. This should have violations

public function testValidateWithNonIntegerField1ValueShouldHaveViolation()
{
    $violations = $this->validator->validate(['field1'=>'NOT_AN_INTEGER']);
    $this->assertHasViolations($violations);
    $this->assertSame('This value should be of type integer.', $violations[0]->getMessage());
}

Results:

PHPUnit 7.2.6 by Sebastian Bergmann and contributors.

.                                                                   1 / 1 (100%)

Time: 191 ms, Memory: 4.00MB

OK (1 test, 3 assertions)


And now one without field1, but with another property:

public function testValidateWithOnlyOtherFieldsShouldPass()
{
    $violations = $this->validator->validate(['anotherField'=>'another value']);
    $this->assertHasNoViolations($violations);
}


PHPUnit 7.2.6 by Sebastian Bergmann and contributors.

.                                                                   1 / 1 (100%)

Time: 191 ms, Memory: 4.00MB

OK (1 test, 2 assertions)


And for good measure I chuck it a null and see what happens:

public function testValidateWithNullIsApparentlyOK()
{
    $violations = $this->validator->validate(null);
    $this->assertHasNoViolations($violations);
}


Not sure this is legit IMO - a null is not a collection as per my constraint definition. But... well... not what I care about right now. It passes:

PHPUnit 7.2.6 by Sebastian Bergmann and contributors.

.                                                                   1 / 1 (100%)

Time: 259 ms, Memory: 4.00MB

OK (1 test, 2 assertions)


OK so at this point I reckon I have done my constraints properly as my tests all pass.

And now the test that replicates our problem:

public function testValidateWithNonTraversableShouldCauseViolation()
{
    $violations = $this->validator->validate('a string');
    $this->assertHasViolations($violations);
}


Note that here I am not passing a collection value at all. I am just passing a string. So this should have a violation, right? Nuh-uh:

PHPUnit 7.2.6 by Sebastian Bergmann and contributors.

E                                                                   1 / 1 (100%)

Time: 194 ms, Memory: 4.00MB

There was 1 error:

1) me\adamcameron\general\test\validation\collectionIssue\CollectionValidatorTest::testValidateWithNonTraversableShouldCauseViolation
Symfony\Component\Validator\Exception\UnexpectedTypeException: Expected argument of type "array or Traversable and ArrayAccess", "string" given

C:\src\php\general\vendor\symfony\validator\Constraints\CollectionValidator.php:37
C:\src\php\general\vendor\symfony\validator\Validator\RecursiveContextualValidator.php:829
C:\src\php\general\vendor\symfony\validator\Validator\RecursiveContextualValidator.php:675
C:\src\php\general\vendor\symfony\validator\Validator\RecursiveContextualValidator.php:118
C:\src\php\general\vendor\symfony\validator\Validator\RecursiveValidator.php:100
C:\src\php\general\src\validation\collectionIssue\CollectionValidator.php:36
C:\src\php\general\test\validation\collectionIssue\CollectionValidatorTest.php:63

ERRORS!
Tests: 1, Assertions: 0, Errors: 1.


So let me get this straight. I am passing validation code a value that ought not validate. And what I get is not a "it dun't validate mate" violation, the validation lib just breaks.

Grumble.

Now this just seems to me to be a testing-101-fail on the part of the Validator library. One must always be able to give the validator any value, and the result should just be "yeah all good", or "nup. And here's why". However as the Symfony mob usually do such solid work, I am kinda wondering if I'm messing something up, and just not seeing it?

What do you think? Even if you don't know PHP or Symfony validation, what would your expectations be here? Or can yous ee the glaring error that I can't see from having looked at this too long?

I will presently ask a question on this on Stack Overflow (a heavily-abridged version of this), but wanted to get some more friendly eyes on it first.

Update

This was bugging my colleague so he went in and checked the underlying Symfony code - something I should have done - and the CollectionValidator is very specifically throwing this exception very early in the piece:

public function validate($value, Constraint $constraint)
{
    if (!$constraint instanceof Collection) {
        throw new UnexpectedTypeException($constraint, __NAMESPACE__.'\Collection');
    }
    if (null === $value) {
        return;
    }
    if (!is_array($value) && !($value instanceof \Traversable && $value instanceof \ArrayAccess)) {
        throw new UnexpectedTypeException($value, 'array or Traversable and ArrayAccess');
    }


That strikes me as very daft, and contrary to the intend of a validator lib. It's been there since the very first commit of that code, as far as I can tell (back in 2010). It's clearly their intent, but the intent is misguided IMO.

Cheers.

--
Adam

Monday, 7 May 2018

In defence of public properties

G'day:
Recently I was re-writing one of our cron job scripts. It's the sort of thing that runs at 2am, reads some stuff from the DB, analyses it, and emails some crap to interested parties. We usually try to avoid complete rewrites, but this code was written... erm... [cough insert some period of time ago when it was apparently OK to write really shit code with no code quality checks and no tests]... and it's unmaintainable, so we're biting the bullet and rewriting coherently whilst adding some business logic changes into it.

The details for the email - to, from, subject, data to build the body from - come from three different sources in total, before sending to the mailer service to be queued-up for sending. So I knocked together a simple class to aggregate that information together:

class SpecialInterestingReportEmail {

    public $from;
    public $to;
    public $subject;
    public $body;

    public static $bodyTemplate = "specialInterestingReportEmail.html.twig";
    
    function __construct($from, $to, $subject, $body) {
        $this->from = $from;
        $this->to = $to;
        $this->subject = $subject;
        $this->body = $body;
    }
}

I then have a factory method in an EmailMessageService (not entirely sure that's the best name for it, but [shrug]) which chucks all the bits and pieces together, and returns an email object:

function createSpecialInterestingReportEmail($special, $interesting) {
    $body = $this-twigService->render(
        SpecialInterestingReportEmail::bodyTemplate,
        [
            "special" => $special,
            "interesting" => $interesting
        ]
    )
    return new SpecialInterestingReportEmail(
        $this->addressesForReport->from,
        $this->addressesForReport->to,
        $this->reportSubject,
        $body
    );
}

I don't think this design is perfect, but in the context of what we're doing it's OK.

Notice:
All interaction I mention below with my colleagues is paraphrased, embellished, or outright changed to make my point. I am not quoting anyone, and take this as a work of fiction. It's also drawn from previous similar conversations I've had on this topic.

One of my code reviewers was horrified:

Code Reviewer: "You can't have public properties! That's bad OOP!"
Me: "Why?"
CR: "You're breaking encapsulation. You need to do it like this [proceeds to tutor me on how to write accessor methods, because, like, I needed a tutorial on design anti-patterns]".
Me: "Why? What are we gaining by making those properties private, just to force us to write a bunch of boilerplate code to then access them? And how is that a good change to make to this code that now exists and is fulfilling the requirement? This is code review... we've already done the planning for this. This class is just a single container to aggregate some data into a conceptual 'email message' object. It doesn't need any behaviour - it doesn't need any methods - it's just for moving data between services".
CR: "But you can't break encapsulation!"
Me: "Why not? Other than parroting dogma read from some OOP 101 book, why is this a problem? And how's it solved by accessor methods?"
CR: "Well someone could directly change the values to be wrong!"
Me: "Well... they probably shouldn't do that then. That'd be dumb. They could equally put the wrong values straight into the constructor too. It'd still be just as wrong. Look... we don't need to validate the data - that seems to be your concern here? - as it's just a script reading known-good values from the DB, and sending them to our email queue. The code's right there. There's no scope for the data to accidentally be wrongified. And if it was... the tests would pick it up anyhow".
CR: "But what if other code starts using this code?"
Me: "What... yer saying 'what it some other application starts using this cronjob as some sort of library?' Why would that happen? This is not a public API. It makes no pretence of being a public API. If anyone started using this as an API, they deserve everything they get".
CR: "But they might".
ME: "OK, let's say some of this code is gonna be needed somewhere else, and we need to make it into a library. At that point in time, we'd extract the relevant code, consider stuff like encapsulation, data hiding, providing a public interface etc. But that would be different code from this lot".
CR: "But you should write all code like it will be used that way".
Me: "Again: why? This code is not going to be used this way. It just isn't. And anyhow, what yer suggesting is YAGNI / coding-for-an-unknown-future anyhow. We don't gain anything for the purposes of the current requirement chucking pointless boilerplate code into these classes. That's not an improvement".

And it went on.

One problem I encounter fairly frequently - both at work and in the wild - is people who will read about some concept, and thenceforth That Concept Is Law. Someone has Said It, therefore it applies to every single situation thereafter. They don't ever seem to bother trying to reason why the "law" was stated in the first place, what about it makes it a good rule, and when perhaps it's not necessary.

I look at these things and think "will this benefit my current task?". Generally it does because these things don't acquire acceptance without scrutiny and sanity-checking. But sometimes, however, it doesn't make sense to follow the dogma.

In this situation: it does not help.

In a different situation, if I was writing a separate API which handled the Email object creation, and other apps were gonna use it, I'd've possibly tightened-up the property access. But only possibly. My position on such things is to be liberal with what one permits to be done with code. If all my theoretical accessor method was gonna achieve was to return a private value... I'd really consider just leaving it public instead, and allow direct access. Why not?

There's a risk that later I might need to better control access to those properties, but... I'd deal with that at the time: these things can be handled as/when. It's even easy enough to have transitionary code from direct-access to accessor-access using __get and __set. I know these are frowned-upon, but in transitionary situations: they're fine. So one could seamlessly patch the API for code already consuming it via direct access with that transitionary approach, and then in the next (or some subsequent ~) version, make the breaking change to require using accessors, eg:

v1.0 - direct access only.
v1.1 - add in transitionary code using __get and __set. Advise that direct access is deprecated and will be removed in the next release. Also add in accessor methods.
v2.0 - remove direct access.

It doesn't even need to be v2.0. Just "some later version". But for the sake of making sure the transitionary code is temporary, better to do sooner rather than later. The thing is that software versioning is there for a reason, so it's OK to only introduce necessary coding overhead when it's needed.

Another thing that occurred to me when I was thinking about this. Consider this code:

$email = [
    "from" => "from@example.com",
    "to" => "to@example.com",
    "subject" => "Example subject",
    "body" => "Example body"
];

$fromAddress = $email["from"];

Perfectly fine. So how come this code is not fine:

$email = new Email(
    "from@example.com",
    "to@example.com",
    "Example subject",
    "Example body"
);

$fromAddress = $email->from;

Why's it OK to access an array directly, but it's not - apparently - OK to give the collection of email bits a name (Email), and otherwise use it the same way?

I can't think of one.

Rules are great. Rules apply sensibly 95% of the time. But when one reads about a rule... don't just stop there... understand the rule. The rules are not there simply for the sake of enabling one to not then think about things. Always think about things.

Righto.

--
Adam

PS: completely happy to be schooled as to how I am completely wrong here. This is half the reason I wrote this.

Saturday, 5 May 2018

PHP: perpetual misuse of array type declarations

G'day:
This has been irking me enough recently to cause me to dust off this old blog.

For a while PHP has had limited argument type checking on function arguments, and this was improved in PHP 7. The docs cover it - Function arguments › Type declarations - so I'll not repeat them here. But we can have this code:

function takesArray(array $a){
    return $a;
}

var_dump(takesArray(["tahi", "rua", "toru", "wha"]));

Output:

array(4) {
  [0]=>
  string(4) "tahi"
  [1]=>
  string(3) "rua"
  [2]=>
  string(4) "toru"
  [3]=>
  string(3) "wha"
}

And if we try to pass something other than an array:

<?php
function takesArray(array $a){
    return $a;
}

try {
    $a = takesArray("NOT_AN_ARRAY");
} catch (\Throwable $t) {
    echo $t;
}

We get this:

TypeError:
Argument 1 passed to takesArray() must be of the type array, string given,
called in [...][...] on line 7 and defined in [...][...]:2
Stack trace:
#0 [...][...](7): takesArray('NOT_AN_ARRAY')
#1 {main}

Fair enough. No complaint there.

What I have an issue with is that now I see a lot of devs slapping "array" as an parameter type in every method signature that takes an array. What's wrong with that you probably ask? Well... in most situations they're doing this, it's not simply an array that is the requirement. It's generally one of these two situations:

function getFirstName(array $name){
    return $name['firstName'];
}

$firstName = getFirstName(["lastName" => "Cameron", "firstName" => "Adam"]);

echo $firstName;

OK, this is a really contrived example, but it's simple and demonstrates the point. Here the function takes an array, and returns the firstName from the array. But note that the requirement of the argument is not "it's an array", as stated. It very specifically needs to be "an associative array that has a key firstName". "array" is not correct here. It's not at all helpful. If one is needing to do this sort of thing, then create a class Name, and specify that. Because that's what you actually want here:

class Name {
    public $lastName;
    public $firstName;
    
    function __construct($lastName, $firstName) {
        $this->lastName = $lastName;
        $this->firstName = $firstName;
    }
}

function getFirstName(Name $name){
    return $name->firstName;
}

The second bogus usage of array as a type declaration is with indexed arrays. Consider this:

function filterOutEarlierDates(array $dates, int $year){
    $cutOff = new \DateTime("$year-01-01");

    $filtered = array_filter($dates, function (\DateTimeInterface $date) use ($cutOff) {
        $diff = $date->diff($cutOff);
        return $diff->invert === 1;
    });
    
    return array_values($filtered);
}

$dobs = [
    new \DateTime("1968-12-20"),
    new \DateTime("1970-02-17"),
    new \DateTime("2011-03-24"),
    new \DateTime("2016-08-17")
];
$cutOff = 2011;

$youngins = filterOutEarlierDates($dobs, $cutOff);

var_dump($youngins);


Here filterOutEarlierDates claims it takes an array. Well it does. Again this lacks the necessary precision for the requirement. It doesn't take an array. It takes - specifically - an array of DateTimeInterface objects. If you give it anything else than that, the code breaks. As type checking is specifically to guard against that, specifying array is simply neither correct or even helpful here.

Specifying array as an argument type check does have a place. When what the function needs actually is an array. And just an array. Any old array.

function array_some(array $array, callable $callback) {
    foreach ($array as $key => $value) {
        $result = $callback($value, $key);
        if ($result) {
            return true;
        }
    }
    return false;
}

$colours = ["whero", "karaka", "kowhai", "kakariki", "kikorangi", "poropango", "papura"];

$containsSixLetterWords = array_some($colours, function ($colour) {
   return strlen($colour) === 6; 
});

var_dump($containsSixLetterWords);

$numbers = [
    "one" => "tahi",
    "two" => "rua",
    "three" => "toru",
    "four" => "wha",
    "five" => "rima",
    "six" => "ono",
    "seven" => "whitu",
    "eight" => "ware",
    "nine" => "iwa",
    "ten" => "tekau"
];

$hasShortKeys = array_some($numbers, function ($number, $key) {
   return strlen($key) <= 3; 
});

var_dump($hasShortKeys);

Here's a PHP equivalent of JS's some method. It takes an array (any array) and a callback, and applies the callback to each array entry until the callback returns true, then it exits. This function correctly claims the first argument value needs to be an array. Without any other sort of qualification: it just needs to be an array.

Actually there's a similar issue with my type checking of the $callback parameter there. It can't be any callable: it needs to be a callable that takes a coupla arguments and returns a boolean. In C# I guess we'd use a Delegate for that, but there's no such construct in PHP. So I should probably settle for giving the argument a clear name ($callback probably isn't good enough here), and dispense with the type check.

But even then I question the merits of type-checking array here. What if instead of just an array, we had a collection object that implements \Iterator, eg:

class ColourCollection implements \Iterator {
    private $position = 0;
    private $colours;  

    public function __construct($colours) {
        $this->position = 0;
        $this->colours = $colours;
    }

    public function rewind() {
        $this->position = 0;
    }

    public function current() {
        return $this->colours[$this->position];
    }

    public function key() {
        return $this->position;
    }

    public function next() {
        ++$this->position;
    }

    public function valid() {
        return isset($this->colours[$this->position]);
    }
}

$rainbow = new ColourCollection($colours);

If we run this variation of the code, we get:

Fatal error:
Uncaught TypeError: Argument 1 passed to array_some() must be of the type array, object given,
 called in [...][...] on line 48 and defined in [...][...]:2
Stack trace:
#0 [...][...](48): array_some(Object(ColourCollection), Object(Closure))
#1 {main}
  thrown in [...][...] on line 2

If we take the type check out: the code works. So how is the array type check really helping us here? It's not.

To be honest I think I'm being a bit edge-case-y with that last example. I don't see a lot of objects that implement the iterator class: we just stick with arrays instead. But it is a legit consideration I guess.

I would use array as a type check in that array_some situation - where the argument actually just needs to be an array, with no special other qualities about it. But this is very rare. In other cases where the argument value needs to be "some specific type or array", then I think it's dead wrong to have an array type check there.

Righto.

--
Adam