Jump to content


Photo

Type declarations for an array of objects


  • Please log in to reply
8 replies to this topic

#1 NotionCommotion

NotionCommotion
  • Members
  • PipPipPip
  • Advanced Member
  • 1,601 posts

Posted 16 May 2018 - 08:05 PM

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

NotionCommotion

#2 kicken

kicken
  • Gurus
  • Wiser? Not exactly.
  • 3,407 posts
  • LocationBonita, FL

Posted 16 May 2018 - 09:10 PM

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]);
    }
}


Did I help you out? Feeling generous? I accept tips via Bitcoin @ 14mDxaob8Jgdg52scDbvf3uaeR61tB2yC7
Kicken's World⦄ ⦃Recycle old CD's

#3 NotionCommotion

NotionCommotion
  • Members
  • PipPipPip
  • Advanced Member
  • 1,601 posts

Posted 17 May 2018 - 02:17 AM

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.


NotionCommotion

#4 requinix

requinix
  • Administrators
  • Impoverished Administrator
  • 9,874 posts
  • LocationWA

Posted 17 May 2018 - 02:26 AM

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.
"Basically, I think the general rule of thumb is: if someone really wants the blood that's inside of your body, and they're like a vampire, or a dracula, or some sort of man-squito, then that's probably okay. A dracula and a man-squito are made for removing things like blood and swords from inside your body. That's basically fine. If something wants to get at your blood and they're, say, some kind of murdersaurus, or maybe a really big frog, that's where the problems start to arise. A really big frog is not made for removing blood, and your blood knows this, which is why it is so vehement about wanting to stay in your body instead of coming out. Unfortunately this will not deter a really big frog because a really big frog is full of things like prizes, and value, and quite a lot of hatred, and it would really rather like to replace any and all of those things with your blood, and basically by any means possible." --slumbermancer

#5 kicken

kicken
  • Gurus
  • Wiser? Not exactly.
  • 3,407 posts
  • LocationBonita, FL

Posted 17 May 2018 - 02:36 AM

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.
Did I help you out? Feeling generous? I accept tips via Bitcoin @ 14mDxaob8Jgdg52scDbvf3uaeR61tB2yC7
Kicken's World⦄ ⦃Recycle old CD's

#6 NotionCommotion

NotionCommotion
  • Members
  • PipPipPip
  • Advanced Member
  • 1,601 posts

Posted 17 May 2018 - 05:29 AM

...

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?


NotionCommotion

#7 requinix

requinix
  • Administrators
  • Impoverished Administrator
  • 9,874 posts
  • LocationWA

Posted 17 May 2018 - 05:54 AM

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.
"Basically, I think the general rule of thumb is: if someone really wants the blood that's inside of your body, and they're like a vampire, or a dracula, or some sort of man-squito, then that's probably okay. A dracula and a man-squito are made for removing things like blood and swords from inside your body. That's basically fine. If something wants to get at your blood and they're, say, some kind of murdersaurus, or maybe a really big frog, that's where the problems start to arise. A really big frog is not made for removing blood, and your blood knows this, which is why it is so vehement about wanting to stay in your body instead of coming out. Unfortunately this will not deter a really big frog because a really big frog is full of things like prizes, and value, and quite a lot of hatred, and it would really rather like to replace any and all of those things with your blood, and basically by any means possible." --slumbermancer

#8 NotionCommotion

NotionCommotion
  • Members
  • PipPipPip
  • Advanced Member
  • 1,601 posts

Posted 17 May 2018 - 02:16 PM

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.


NotionCommotion

#9 requinix

requinix
  • Administrators
  • Impoverished Administrator
  • 9,874 posts
  • LocationWA

Posted 17 May 2018 - 10:07 PM

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?"
"Basically, I think the general rule of thumb is: if someone really wants the blood that's inside of your body, and they're like a vampire, or a dracula, or some sort of man-squito, then that's probably okay. A dracula and a man-squito are made for removing things like blood and swords from inside your body. That's basically fine. If something wants to get at your blood and they're, say, some kind of murdersaurus, or maybe a really big frog, that's where the problems start to arise. A really big frog is not made for removing blood, and your blood knows this, which is why it is so vehement about wanting to stay in your body instead of coming out. Unfortunately this will not deter a really big frog because a really big frog is full of things like prizes, and value, and quite a lot of hatred, and it would really rather like to replace any and all of those things with your blood, and basically by any means possible." --slumbermancer




0 user(s) are reading this topic

0 members, 0 guests, 0 anonymous users