Jump to content
Sign in to follow this  
NotionCommotion

Type declarations for an array of objects

Recommended Posts

I have a class who's constructor must save an array of objects.  One option is as follows:

 



class Collections implements CollectionsInterface
{
    protected $stack=[];
    protected $nodeClass; //Fully qualified class name


    public function __construct($nodeClass, array $nodes=[]) {
        $this->nodeClass=$nodeClass;
        foreach($nodes as $node) {
            $this->stack[]=new $nodeClass($node);
        }
    }
}


I am assuming doing so is not the way to do so, but instead should just pass it an array of the objects, correct?  But if so, how do I enfource doing so?

 

I can do this:



interface CollectionsInterface
{
    public function __construct(array $arrayOfNodes);
}


and I can do this:


interface CollectionsInterface
{
    public function __construct(Node $node);
}

but instead I want to enforce that an array of Node objects is provided.

 

Is this possible?  If so, how?

 

If not, any recommended workarounds?  Don't know if it is a good idea, but was thinking that instead of requiring an array of given object types, I would require a given object (maybe that extends ArrayIterator?) which acts like one.

 

Thank you

Share this post


Link to post
Share on other sites

Afaik, in PHP there is no way to specify an array of X, just either X or generic array. If you include docblock comments, you can document a parameter as being an array of X using the notation X[], for example:

/**
 * Set available categories
 *
 * @param Category[] $categories
 */
public function setCategories(array $categories){
  //...
}
A decent IDE will pick up that hint and use it to type check your code, but PHP itself will only enforce that the parameter is an array.

 

If you really want to have some type checking at runtime then you could do as you said and create a collection class, either one specific to the type you want or a generic one that does a type check and throws an exception on failure.

 

For example (untested):

class ObjectArray implements ArrayAccess {
    private $type;
    private $storage;

    public function __construct($type){
        $this->type = $type;
        $this->storage = [];
    }

    public function offsetExists($offset){
        return array_key_exists($offset, $this->storage);
    }

    public function offsetGet($offset){
        return $this->storage[$offset];
    }

    public function offsetSet($offset, $value){
        if (!is_a($value, $this->type)){
            throw new \InvalidArgumentException('Value must be instance of ' . $this->type);
        }
        
        $this->storage[$offset] = $value;
    }

    public function offsetUnset($offset){
        unset($this->storage[$offset]);
    }
}

Share this post


Link to post
Share on other sites

Thanks kicken,

 

Sounds like you don't feel it is absolutely necessary.  My main reason to do so is just to form good habits, and if there is not a straight forward approach, I will likely not do so.

Share this post


Link to post
Share on other sites

Remember that enforcing this constraint costs time. If you're confident that the array will be valid then you don't need to spend resources on making sure it does.

 

I assume you've heard about assertions at some point? Consider something like:

function that_array_contains_only(array $array, callable $check) {
	foreach ($array as $key => $value) {
		if (!$check($value, $key)) {
			return false;
		}
	}
	return true;
}
class Collections implements CollectionsInterface
{
	protected $stack=[];
	protected $nodeClass; //Fully qualified class name


	public function __construct($nodeClass, array $nodes=[]) {
		assert(that_array_contains_only($nodes, function($a) use ($nodeClass) { return $a instanceof $nodeClass; }));
		$this->nodeClass=$nodeClass;
		foreach($nodes as $node) {
			$this->stack[]=new $nodeClass($node);
		}
	}
}
With assertions enabled (development) it will check the array, and with them disabled (production) there is no check but it doesn't cost you any CPU time.

 

 

And for the record: it's true, PHP doesn't have an array-of type hint.

Share this post


Link to post
Share on other sites

I just document the type in docblock comments and consider that good enough. Having those hints is enough for PHPStorm to do proper auto-completion and issue warnings when it suspects a type mismatch.

 

If PHP supported such type annotations natively then It'd be worth using them, but as it is currently implementing collection classes seems like a waste of effort to me.

 

Apparently this feature was proposed but rejected.

Share this post


Link to post
Share on other sites

...

class Collections implements CollectionsInterface
{
	protected $stack=[];
	protected $nodeClass; //Fully qualified class name


	public function __construct($nodeClass, array $nodes=[]) {
		assert(that_array_contains_only($nodes, function($a) use ($nodeClass) { return $a instanceof $nodeClass; }));
		$this->nodeClass=$nodeClass;
		foreach($nodes as $node) {
			$this->stack[]=new $nodeClass($node);
		}
	}
}
...

 

 

My intention with my first option was to pass the class name (not an object or an array of objects) plus a normal array to Collection's constructor, iterate over the array, and create new Node objects.  I have no reason to think so, but it just seemed like the wrong way of doing so, and that I should instead have the application create both the Node objects first and then inject these Node objects when creating the Collection object.  Was this apparent?  Is it frowned upon, difficult to test, or create unexpected problems by passing the class name instead of an object?

Share this post


Link to post
Share on other sites

I'll admit, I didn't pay too close attention to the code when I added that line so... yeah... I'm really terrible about doing that.

assert(that_array_contains_only($nodes, function($a) use ($nodeClass) { return $a == $nodeClass || is_subclass_of($a, $nodeClass); }));
It doesn't sound right to me either. $nodes should be Node objects before it gets to Collection. You could provide a helper method in Node or Collection to... help... but IMO Collection should only have to care that it holds valid objects - not the nature of those objects or how they're created.

Share this post


Link to post
Share on other sites

It doesn't sound right to me either. $nodes should be Node objects before it gets to Collection. You could provide a helper method in Node or Collection to... help... but IMO Collection should only have to care that it holds valid objects - not the nature of those objects or how they're created.

 

 

You answered exactly how I would have:  I doesn't sound right and in my opinion...

 

While I trust your opinion much more than mine, it would be nice to know why it isn't right.

Share this post


Link to post
Share on other sites

It's just such a vague thing, "why". I know of the Collection concept from strictly-typed OOP languages, and they don't do things like automatically create instances from data. More specific types of collections might but it looks like you're going for something generic, and it's just because you're using PHP that you're able to do stuff like "new $nodeClass". If it weren't for that feature then you would have to do it normally, and since I don't like using variable variables that means no automatic construction.

 

Along those lines is the "the Collection class shouldn't have to know how to construct whatever objects it's storing" argument. You're making a class to hold stuff: it should know about inheritance and it should validate items in the collection and stuff, but exactly how those objects were created is/should be irrelevant. Instead, if you need a way to create an array of a class then that class should provide the method - if it makes sense to, like because doing so is a common use case. Then you pass that constructed array to Collection and it validates the members:

$this->nodeClass = $nodeClass;
$this->addMany($nodes); // using the obvious implementation
Basically, for me it boils down to "what do better designed and more mature languages do?"

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
Sign in to follow this  

×

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.