php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #63092 IteratorAggregate interface should allow for getIterator to be static
Submitted: 2012-09-14 21:37 UTC Modified: 2013-03-15 14:52 UTC
From: slogger at lavabit dot com Assigned:
Status: Wont fix Package: Class/Object related
PHP Version: Irrelevant OS: All
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: slogger at lavabit dot com
New email:
PHP Version: OS:

 

 [2012-09-14 21:37 UTC] slogger at lavabit dot com
Description:
------------
See:
http://www.php.net/manual/en/class.iteratoraggregate.php

If following common architectures/design patterns, the following case is quite common:

// Given a class Thing where the constructor sets a 'name' property
// and implements IteratorAggregate:
dog = new Thing('dog');
cat = new Thing('cat');
$itty = Thing::getIterator();
foreach($itty as $thing){
  echo "{$thing->name} \n";
}
// dog
// cat

However, the current IteratorAggregate does not allow for getIterator() to be static. This seems like a big flaw. Obviously nothing prevents creation of such a static method and NOT implementing IteratorAggregate, but that seems kind of wrong.

Obviously the issue of how getIterator should work if it can be either static or non-static is kind of fugly, but I tend to think that it ought to have been static to begin with -- usually you want to iterate types/collections, not an individual thing/instance.

Test script:
---------------
// Obviously this is a crude example, but it shows the error.
// In a more complex example, $things might be some sort of Iterator, rather
// than just an array.

class Thing implements IteratorAggregate{
   public static $things = array();
   public function __construct($name){
      $this->name = $name;
      static::$things[] = $this;
   }
   public static function getIterator(){
      return static::$things;
   }
}

$dog = new Thing('dog');

foreach(Thing::getIterator() as $thing){
   echo $thing->name;
}

Expected result:
----------------
dog

Actual result:
--------------
Fatal error: Cannot make non static method IteratorAggregate::getIterator() static in class Thing in php shell code on line 1

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2012-09-18 00:56 UTC] mail+php at requinix dot net
>If following common architectures/design patterns, the following case is quite
>common:
Not that I've ever seen. You're essentially asking for a built-in registry 
pattern where one doesn't make sense. There are just too many problems trying to 
do it that way.

This isn't the place for discussion, but if you want magic Iterator behavior then 
a factory would be a better option.

$factory = new ThingFactory(); // implements IteratorAggregate
$dog = $factory->create("dog"); // create() adds the object to $this->things
$cat = $factory->create("cat");

foreach($factory as $thing) {
    echo "I was used to create a {$thing->name}\n";
}
 [2012-09-18 04:10 UTC] slogger at lavabit dot com
I disagree with the registry pattern fitting what I'm describing because one of the main points of an iterator is to avoid having a registry with all objects in memory at once. That's also why I didn't really think using an array in my example was appropriate. 

A do agree that factory seems similar at first blush, but it's actually almost the exact reverse of the relationship I'd like to see -- rather than a factory creating products, it's more of a registration/manager pattern, where, say, you create 'fans' or 'players' that automatically or naturally register with a 'team' in some kind of backing store, and then fan::getIterator pulls through an iterator via instantiating or grabbing a 'team' iterator for 'fans' rather than 'fans + players', the bonus being that team iterator is more or less invisible, and can manage itself so that the iterator doesn't have a massive list in memory and can handle either 'players' or 'fans'.

Anyway, yeah, my simple example probably doesn't make it clear why I would have wanted the particular behavior, though I think you're right, that maybe it's less common a pattern than you'd think, though I'd say it's essentially the same pattern as many pagination systems or relational-object systems, or other systems where you can only look at a subset of something at a time follow.

All that said, it's nothing I can't just do myself, and you're right, it is maybe is more of a pattern where, when full blown, things have a reference to or encapsulate a manager class, rather than just being able to get a specialized iterator, though I think you still run into it depending on whether the manager is a singleton, as you may want to proxy Thing::getIterator() to ThingManagerInstance->getIterator(). Obviously you can do exactly that, just, say, Thing::filter() or Thing::all() proxying to thingManagerInstance->all() or thingManagerInstance->filter(), Though I guess you could use magic methods to make it so that Thing::manager->getIterator() or Thing::objects->getIterator() worked. I guess the example I'm illustrating might be Player::getIterator() mapping to Player::teamManager->filteredIterator() or something...

Anyway, this bug isn't the end of the world as you can do exactly the things I want without trouble, you just can't say they implement IteratorAggregate. So yeah, take this as just a note that I think there can be legit reasons why you might want getIterator() to be static.

Eh. Hopefully this made sense and I'm not saying something incredibly stupid or obfuscating at all.
 [2013-03-15 14:52 UTC] nikic@php.net
-Status: Open +Status: Wont fix
 [2013-03-15 14:52 UTC] nikic@php.net
IteratorAggregate::getIterator() is there so thing work "automatically" in foreach, without the need to write foreach($foo->getIterator() as ...).

In your case you do not want this, so you shouldn't be using this interface. You have some completely different use case that just happens to use the same name. Marking as Won't Fix.
 
PHP Copyright © 2001-2021 The PHP Group
All rights reserved.
Last updated: Sun May 09 08:01:23 2021 UTC