NotionCommotion Posted May 16, 2018 Share Posted May 16, 2018 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 Quote Link to comment Share on other sites More sharing options...
kicken Posted May 16, 2018 Share Posted May 16, 2018 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]); } } Quote Link to comment Share on other sites More sharing options...
NotionCommotion Posted May 17, 2018 Author Share Posted May 17, 2018 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. Quote Link to comment Share on other sites More sharing options...
requinix Posted May 17, 2018 Share Posted May 17, 2018 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. Quote Link to comment Share on other sites More sharing options...
kicken Posted May 17, 2018 Share Posted May 17, 2018 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. Quote Link to comment Share on other sites More sharing options...
NotionCommotion Posted May 17, 2018 Author Share Posted May 17, 2018 ... 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? Quote Link to comment Share on other sites More sharing options...
requinix Posted May 17, 2018 Share Posted May 17, 2018 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. Quote Link to comment Share on other sites More sharing options...
NotionCommotion Posted May 17, 2018 Author Share Posted May 17, 2018 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. Quote Link to comment Share on other sites More sharing options...
requinix Posted May 17, 2018 Share Posted May 17, 2018 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 implementationBasically, for me it boils down to "what do better designed and more mature languages do?" Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.