[wp-trac] [WordPress Trac] #45825: Use of same loop variable in inner foreach loop can be error-prone

[WordPress Trac]

#45825: Use of same loop variable in inner foreach loop can be error-prone
------------------------------+-------------------------------
 Reporter:  subrataemfluence  |       Owner:  (none)
     Type:  defect (bug)      |      Status:  new
 Priority:  low               |   Milestone:  Future Release
Component:  Upgrade/Install   |     Version:  5.0.2
 Severity:  trivial           |  Resolution:
 Keywords:  has-patch         |     Focuses:  coding-standards
------------------------------+-------------------------------

Comment (by madivad):

 Replying to [ticket:45825 subrataemfluence]:
 You could edit the ticket to remove a typo (the second reference to
 `$column` has `$columns`, hence the only reason why I delved into the code
 to actually check what was there. `$columns` would mean that it is a new
 variable and hence not required to be changed.

 However, the typo is in your original post and they are in fact the same
 variable name.

 I do submit it is irrelevant and given the nature of scoping within `PHP`,
 the inner reference to `$column` cannot alter or modify the outer loop
 variable as it is not passed by reference (the inner would need to be
 `&$column`). Given this (that the inner foreach variable can never alter
 the outer loop) it's a non-issue and the code could stand as-is, however,
 for clarity there is no reason this should not be committed.

 > It is not a good practice and not recommended to use the same loop
 variable in both outer and inner loop. In `wp-admin/install-helper.php`
 function `maybe_drop_column` does the following:
 >
 >
 > {{{
 > function maybe_drop_column( $table_name, $column_name, $drop_ddl ) {
 >    ...
 >    foreach( ...  as $column ) {
 >      ...
 >      foreach( ... as $columns ) {
 >        ...
 >      }
 >    }
 > }
 > }}}
 >
 >
 > I would recommend to change the inner loop variable to
 `$recheck_column`.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/45825#comment:2>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform