r101244 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r101243‎ | r101244 | r101245 >
Date:02:04, 29 October 2011
Author:reedy
Status:reverted (Comments)
Tags:
Comment:
Remove more assignments in conditionals (while each to foreach)
Modified paths:
  • /trunk/phase3/includes/MagicWord.php (modified) (history)
  • /trunk/phase3/includes/diff/DairikiDiff.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/MagicWord.php
——@@ -754,7 +754,7 @@
755755 */
756756 function parseMatch( $m ) {
757757 reset( $m );
758 - while ( list( $key, $value ) = each( $m ) ) {
 758+ foreach ( $m as $key => $value ) {
759759 if ( $key === 0 || $value === '' ) {
760760 continue;
761761 }
Index: trunk/phase3/includes/diff/DairikiDiff.php
——@@ -371,7 +371,7 @@
372372 }
373373 $matches = $ymatches[$line];
374374 reset( $matches );
375 - while ( list( , $y ) = each( $matches ) ) {
 375+ foreach ( $matches as $y ) {
376376 if ( empty( $this->in_seq[$y] ) ) {
377377 $k = $this->_lcs_pos( $y );
378378 assert( $k > 0 );
——@@ -379,7 +379,7 @@
380380 break;
381381 }
382382 }
383 - while ( list ( , $y ) = each( $matches ) ) {
 383+ foreach ( $matches as $y ) {
384384 if ( $y > $this->seq[$k -1] ) {
385385 assert( $y < $this->seq[$k] );
386386 // Optimization: this is a common case:
——@@ -493,7 +493,7 @@
494494 // Use the partitions to split this problem into subproblems.
495495 reset( $seps );
496496 $pt1 = $seps[0];
497 - while ( $pt2 = next( $seps ) ) {
 497+ foreach ( $seps as $pt2 ) {
498498 $this->_compareseq ( $pt1[0], $pt2[0], $pt1[1], $pt2[1] );
499499 $pt1 = $pt2;
500500 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r102096Partial revert to r101244 due to next() weirdnessreedy14:10, 5 November 2011
r102591Reverted r101244 changes - these are not equivalent and may have been causing...aaron22:59, 9 November 2011

Comments

#Comment by Brion VIBBER (talk | contribs)   22:41, 1 November 2011

There's a next() call inside the look in parseMatch. PHP docs warn against relying on the array pointer value during or after foreach() so this may or may not be undefined behavior... in theory foreach operates on a copy of the array unless you explicitly referenced it, so i'm not sure it would actually be advancing the pointer on the original....?

#Comment by Aaron Schulz (talk | contribs)   22:52, 1 November 2011

Using next() in foreach gives wierd results: 

<?php
$x = array(1,2,3,4,5,6,7,8,9);
foreach( $x as $n => $val ) {
	if ( $val == 6 ) {
		var_dump( $val );
		var_dump( next( $x ) );
		return;
	}
}

Gives: 

int(6) int(8)
#Comment by Reedy (talk | contribs)   23:44, 1 November 2011

So the simplest option is to revert the change for that loop.

Else, a for loop, and for the next actually use $array[$i+1]..

Status & tagging log