Page 1 of 1

Iterator and SImpleIterator combined

Posted: Tue May 06, 2003 1:48 pm
by jason
The idea was based on conversations with Harry Fuecks ( http://www.phppatterns.com/index.php/ar ... ew/50/1/1/ ) and various postings on the SitePoint forums ( http://www.sitepointforums.com/showthre ... adid=93442 ).

Simply put, I believe each of the main posts, Harry included, is wrong. The concept stems from my work with the Eclipse library, and the Iterator object presented here:

Code: Select all

<?php
class Iterator {
    /***
     * Create a new iterator that's immediately ready for use.  Normally,
     * the constructor calls <code>reset()</code>.
     ***/
    function Iterator(& $container) {}

    /***
     * Initialize this iterator.
     * @returns void
     ***/
    function reset() {}

    /***
     * Advance the internal cursor to the next object. The behavior of this
     * method is undefined if <code>isValid()</code> returns <code>false</code>.
     * @returns void
     ***/
    function next() {}

    /***
     * Check if the iterator is valid
     * @returns bool
     ***/
    function isValid() {}

    /***
     * Return a reference to the current object. The behavior of this method
     * is undefined if <code>isValid()</code> returns <code>false</code>.
     * @returns mixed
     ***/
    function &getCurrent() {}
}
?>
What's interesting to note is that it's a complete Iterator as described in the book of four. Problem being, it doesn't take into account the fact that PHP is a very dynamic language, and doesn't need to rely on methods built for languages like C++ and Java (strongly typed languages).

Harry proposed this option:

Code: Select all

<?php
/**
 * Defines the interface for concrete Iterators
 *
 * @interface
 */
class SimpleIterator {
    /**
     * Returns the current element from the collection
     * and moves the internal pointer forward one
     *
     * @return mixed
     */
    function fetch() {
        die ('SimpleIterator::fetch must be implemented');
    }

    /**
     * Returns the number of elements in the collection
     *
     * @return int
     */
    function size() {
        die ('SimpleIterator::size must be implemented');
    }

    /**
     * Resets the collection pointer to the start
     *
     * @return void
     */
    function reset() {
        die ('SimpleIterator::reset must be implemented');
    }
}
?>
This option is equally as bad. First, I don't want a quantifier in my object. SimpleIterator serves the same purpose as the Iterator object, except it's meant to be easier.

The SimpleIterator doesn't solve the problem, it just shifts the potential solution from the potential problem.

Really, the problem is this. Why should a programmer do this:

Code: Select all

<?php

for ( $it2->reset(); $it2->isValid(); $it2->next() ) {
	$row = $it2->getCurrent();
	echo 'IT2 -> ',$row[0],"\n";
}

?>
When they should be able to do this:

Code: Select all

<?php

while ( $it->next() ) {
	$row = $it->getCurrent();
	echo 'IT -> ',$row[0],"\n";
}

?>
Of course, $it is considered to be the iterator object.

The problem wasn't with the interace, but with how the object worked interanally.

So I left the Iterator object the way it was, and just added:

Code: Select all

<?php

	/**
	* The current iteration value
	* @type mixed
	*/
	var $current_iteration;
	
	/**
	* The result of the <code>next()</code> call
	* @type bool
	*/
	var $next_result = null;

?>
And then, for object implementing the Iterator pattern, they could us these variables. Of course, you could assume a contract, and just create a new object and hard code these variables in rather in include the entire Iterator into your code.

Anyways, looking at QueryIterator, I made the necessary changes to work with the new Iterator.

Code: Select all

<?php

class QueryIterator extends Iterator 
{
	// DATA MEMBERS
	
	/***
	* The query to iterator over (a <code>QueryResult</code> or a
	* <code>Query</code>)
	* @type QueryResult
	***/
	var $queryResult;
	
	/***
	* How each row should be retrieved. Should be <code>ECLIPSE_DB_BOTH</code>,
	* <code>ECLIPSE_DB_ASSOC</code> or <code>ECLIPSE_DB_NUM</code>
	* @type int
	***/
	var $rowType;
	
	/***
	* The index of the current row
	* @type int
	***/
	var $index;
	
	/***
	* The total number of rows in the query
	* @type int
	***/
	var $total;
	
	
	// CREATORS
	
	/***
	* Construct a new <code>QueryIterator</code>-object
	* @param $queryResult the <code>QueryResult</code> to iterate over
	***/
	function QueryIterator(&$queryResult, $rowType = ECLIPSE_DB_BOTH) 
	{
		$this->queryResult =& $queryResult;
		$this->rowType     =  $rowType;
		$this->reset();
	}
	
	// MANIPULATORS
	
	/***
	* @returns void
	***/
	function reset() 
	{
		$this->index = -1;
	}
	
	/***
	* @returns void
	***/
	function next() 
	{
		$this->index++;
		$this->next_result = ( $this->current_iteration =& $this->queryResult->getRow($this->index, $this->rowType) );
		return $this->next_result;
	}
	
	// ACCESSORS
	
	/***
	* @returns bool
	***/
	function isValid() 
	{
		if ( $this->next_result == null ) return $this->next();
		return $this->next_result;
	}
	
	/***
	* Return a reference to the current row of the <code>QueryResult</code>
	* @returns array
	***/
	function &getCurrent() 
	{
		return $this->current_iteration;
	}
}

?>
Basically, the Iterator advances on next(), which makes sense. It also returns a boolean value of false if the next() is unsuccessful, or true if it is. isValid() simply returns that same value. This means you can use both methods of iteration, the normal, classic Iterator approach or the SimpleIterator approach, and you don't have to worry about rewriting code to use either Iterator (my main reason for doing this).

Code: Select all

<?php

include 'MyDatabase.php';
include 'QueryIterator.php';

$db = &new MyDatabase('database', 'host');
$db->connect('username','password');
$result =& $db->query("SHOW TABLES");

$it = &new QueryIterator($result);
$it2 = &new QueryIterator($result);
while ( $it->next() ) {
	$row = $it->getCurrent();
	echo 'IT -> ',$row[0],"\n";
}

for ( $it2->reset(); $it2->isValid(); $it2->next() ) {
	$row = $it2->getCurrent();
	echo 'IT2 -> ',$row[0],"\n";
}

?>
As you can see there, both methods will work fine.

Thoughts, comments, flames?

Posted: Wed May 07, 2003 9:42 pm
by hob_goblin
I like the way you stated the reason for doing this, to make it easier to use and without rewriting code, as thats what I think one of the main points of OOP is. Many people do not realize that the classes were meant to be altered without having other coding changes.

I, however, would probably take the simpleiterator approach, except my class would have a $element_contents variable which would store the contents of the current element. My fetch() function would grab the contents of the element, store it in the variable, and increment an internal counter so the next element would be fetched upon the next run of the loop. Other than that, it would be the same.

I do like your approach better than both of the other ones, though. And the regular iterator seems pointless to me.

Posted: Thu May 08, 2003 4:15 am
by volka
There are two problems I see.
The first you pointed out yourself so you probably gave it serious considerations. Therefor this is simply another opinion against adding $current_iteration and $next_result to the base-declaration. Since php does not provide member protection (yet?) things are getting worse more quickly than in other languages if you define optional parameters of unknown state. Some implementations may find it difficult to maintain these members if you allow iterators of mutable collections or subclassing of a bi-directional iterator. Therefor I'd prefer to keep it a plain interface declaration. Helpful standard implementations can be provided in a subclass (e.g. something like class IteratorImpl extends Iterator)

The second matter is
/***
* @returns void
***/
function reset()
I'm not sure wether a basic iterator must be forced implement this behaviour, some underlaying collections may want to keep control of it (or reject it at all). At least the function should be allowed to state possible error conditions in a return value.

Making it possible to use the iterator in a simple while-loop, of course, is very well and I totally agree with not taking size() into the declaration; it's not the responsibilty of a basic iterator to know the amount of elements it will provide.

Posted: Thu May 08, 2003 4:07 pm
by saberworks
Why even get that complicated? Why not just:

Code: Select all

<?php
$it = & new Iterator;
while($row = $it->get_next()) {
    echo $row['blah'];
}
?>

Posted: Sun May 11, 2003 2:05 pm
by HarryF
Starting from the first last;

Code: Select all

<?php 
$it = & new Iterator; 
while($row = $it->get_next()) { 
    echo $row['blah']; 
} 
?>
The problem there is (assuming the get_next() method really gets the next result), how do you get the very first result?

Jason - agree your approach is more solid. With what I'm doing, the IndexedArrayIterator in particular will be trouble if it's just an array of boolean values - the moment a value is false iteration stops. Note the intention of the size() method is not to meant to prevent this from happening but instead as a useful utility to help with rendering results - didn't really discuss that in the article. Perhaps that class should be renamed StrutIterator or something similar to imply it's only meant for use with arrays of arrays or arrays of objects, the first array always being indexed not associative.

Posted: Mon May 12, 2003 3:27 pm
by saberworks
It's better to work off what will happen MOST of the time (IMO). Instead of get_next(), use get() and it will get the next 'expected' just like mysql_fetch_array() or something. I think the problem is that many people over-think and over-engineer a solution.

Posted: Thu May 15, 2003 2:37 pm
by BDKR
It's my opinion that this is the kind of thing that should be an engine level implementation. Isn't PHP5 going to have iterators via an extension?

Cheers,
BDKR