How to update code that uses deprecated each()-CodePudding

I'm trying to update a piece of old code (a menu builder class). I've updated everything else but I'm stuck at a line that uses the each() function. I did read some of the previous threads but this particular instance is too complex for me to figure out how to change. Here it is:

while ( $loop && ( ( $option = each( $children[$parent] ) ) || ( $parent > $root_id ) ) )

This is the entire function where the above line occurs:

function get_menu_html($lang, $root_id = 0 )
{
    $this->html  = array();
    $this->items = $this->get_menu_items($lang);
    
    foreach ( $this->items as $item )
        $children[$item['sectionParentID']][] = $item;
    
    // loop will be false if the root has no children (i.e., an empty menu!)
    $loop = !empty( $children[$root_id] );
    
    // initializing $parent as the root
    $parent = $root_id;
    $parent_stack = array();
    
    // HTML wrapper for the menu (open)
    $this->html[] = '<ul>';
    
    while ( $loop && ( ( $option = each( $children[$parent] ) ) || ( $parent > $root_id ) ) )
    {
        if ( $option === false )
        {
            $parent = array_pop( $parent_stack );
            
            // HTML for menu item containing children (close)
            $this->html[] = str_repeat( "\t", ( count( $parent_stack )   1 ) * 2 ) . '</ul>';
            $this->html[] = str_repeat( "\t", ( count( $parent_stack )   1 ) * 2 - 1 ) . '</li>';
        }
        elseif ( !empty( $children[$option['value']['sectionID']] ) )
        {
            $tab = str_repeat( "\t", ( count( $parent_stack )   1 ) * 2 - 1 );
            
            // HTML for menu item containing children (open)
            $this->html[] = sprintf(
                '%1$s<li><a href="%2$s">%3$s</a>',
                $tab,   // %1$s = tabulation
                $option['value']['sectionPage'],   // %2$s = sectionPage (URL)
                $option['value']['sectionLabel']   // %3$s = title
            ); 
            $this->html[] = $tab . "\t" . '<ul >';
            
            array_push( $parent_stack, $option['value']['sectionParentID'] );
            $parent = $option['value']['sectionID'];
        }
        else
            // HTML for menu item with no children (aka "leaf") 
            $this->html[] = sprintf(
                '%1$s<li><a href="%2$s">%3$s</a></li>',
                str_repeat( "\t", ( count( $parent_stack )   1 ) * 2 - 1 ),   // %1$s = tabulation
                $option['value']['sectionPage'],   // %2$s = sectionPage (URL)
                $option['value']['sectionLabel']   // %3$s = title
            );
    }

CodePudding user response：

I went for a foreach starting with a check for the 3rd while loop condition.

$this->html  = array();
$this->items = $this->get_menu_items();

foreach ($this->items as $item)
    $children[$item['parent_id']][] = $item;

// loop will be false if the root has no children (i.e., an empty menu!)
$loop = !empty($children[$root_id]);

// initializing $parent as the root
$parent = $root_id;
$parent_stack = array();

// HTML wrapper for the menu (open)
$this->html[] = '<ul>';
if ($loop) {
    foreach ($children[$parent] as $option) {
        if ($parent > $root_id) {
            break; // exit loop
        }
        if ($option === false) {
            $parent = array_pop($parent_stack);

            // HTML for menu item containing childrens (close)
            $this->html[] = str_repeat("\t", (count($parent_stack)   1) * 2) . '</ul>';
            $this->html[] = str_repeat("\t", (count($parent_stack)   1) * 2 - 1) . '</li>';
        } elseif (!empty($children[$option['id']])) {
            $tab = str_repeat("\t", (count($parent_stack)   1) * 2 - 1);

            // HTML for menu item containing childrens (open)
            $this->html[] = sprintf(
                '%1$s<li><a href="%2$s">%3$s</a>',
                $tab,   // %1$s = tabulation
                $option['link'],   // %2$s = link (URL)
                $option['title']   // %3$s = title
            );
            $this->html[] = $tab . "\t" . '<ul >';

            array_push($parent_stack, $option['parent_id']);
            $parent = $option['id'];
        } else
            // HTML for menu item with no children (aka "leaf") 
            $this->html[] = sprintf(
                '%1$s<li><a href="%2$s">%3$s</a></li>',
                str_repeat("\t", (count($parent_stack)   1) * 2 - 1),   // %1$s = tabulation
                $option['link'],   // %2$s = link (URL)
                $option['title']   // %3$s = title
            );
    }

    // HTML wrapper for the menu (close)
    $this->html[] = '</ul>';

    return implode("\r\n", $this->html);
}

Tested with:

[ 
    ['id'=>1, 'parent_id'=>0, 'title'=>'title 1','link'=>'link 1','position'=>'0'],
    ['id'=>2, 'parent_id'=>1, 'title'=>'title 2','link'=>'link 2','position'=>'0'],
    ['id'=>3, 'parent_id'=>1, 'title'=>'title 3','link'=>'link 3','position'=>'0'],
    ['id'=>4, 'parent_id'=>0, 'title'=>'title 4','link'=>'link 4','position'=>'0'],
];

CodePudding user response：

As a general rule, if you need to move away from each($ar) you can usually use [key($ar),current($ar)] as a drop in replacement but then you need to move the pointer within the loop. You usually just need to call next($ar) within the loop and break once you run out of reading room such as breaking when the key is null. Then just make sure the array becomes set to false when it runs out of room.

This type of approach can get ugly very quickly though due to the extra code...

So without really paying much attention to your code, after this update, your revised code would like look:

function get_menu_html($lang, $root_id = 0 )
{
    $this->html  = array();
    $this->items = $this->get_menu_items($lang);
    
    foreach ( $this->items as $item )
        $children[$item['sectionParentID']][] = $item;
    
    // loop will be false if the root has no children (i.e., an empty menu!)
    $loop = !empty( $children[$root_id] );
    
    // initializing $parent as the root
    $parent = $root_id;
    $parent_stack = array();
    
    // HTML wrapper for the menu (open)
    $this->html[] = '<ul>';
    
    while ( $loop && ( ( $option = [key($children[$parent]),current($children[$parent])] ) || ( $parent > $root_id ) ) )
    {
        if($option[0] === null){$option = false;} // replicate each's behavior for after last element
        next($children[$parent]);
        if ( $option === false )
        {
            $parent = array_pop( $parent_stack );
            
            // HTML for menu item containing children (close)
            $this->html[] = str_repeat( "\t", ( count( $parent_stack )   1 ) * 2 ) . '</ul>';
            $this->html[] = str_repeat( "\t", ( count( $parent_stack )   1 ) * 2 - 1 ) . '</li>';
            break;
        }
        elseif ( !empty( $children[$option['value']['sectionID']] ) )
        {
            $tab = str_repeat( "\t", ( count( $parent_stack )   1 ) * 2 - 1 );
            
            // HTML for menu item containing children (open)
            $this->html[] = sprintf(
                '%1$s<li><a href="%2$s">%3$s</a>',
                $tab,   // %1$s = tabulation
                $option['value']['sectionPage'],   // %2$s = sectionPage (URL)
                $option['value']['sectionLabel']   // %3$s = title
            ); 
            $this->html[] = $tab . "\t" . '<ul >';
            
            array_push( $parent_stack, $option['value']['sectionParentID'] );
            $parent = $option['value']['sectionID'];
        }
        else
            // HTML for menu item with no children (aka "leaf") 
            $this->html[] = sprintf(
                '%1$s<li><a href="%2$s">%3$s</a></li>',
                str_repeat( "\t", ( count( $parent_stack )   1 ) * 2 - 1 ),   // %1$s = tabulation
                $option['value']['sectionPage'],   // %2$s = sectionPage (URL)
                $option['value']['sectionLabel']   // %3$s = title
            );
    }

edit: note the above code doesn't work since it references $option['value'] from the each rather than $option[1] from the each. Will need to adjust that in the code.

You might also want to refactor the code. But getting it working, without the deprecated code, is a good first step.

This type of approach is usually very ugly though. Best to refactor and use a foreach loop of some sort specific to the code's logic.

Edit:
Here's an isolated example I'm working on from the link that was provided to the original code:

<?php

/**
 * Generate HTML for multi-dimensional menu from MySQL database
 * with ONE QUERY and WITHOUT RECURSION 
 * @author J. Bruni
 */
class MenuBuilder
{

    /**
     * Menu items
     */
    var $items = array();

    /**
     * HTML contents
     */
    var $html  = array();



    /**
     * Get all menu items from database
     */
    function get_menu_items()
    {
        // Change the field names and the table name in the query below to match tour needs
        return [
                ['id'=>1, 'parent_id'=>0, 'title'=>'title 1','link'=>'link 1','position'=>'0'],
                ['id'=>2, 'parent_id'=>1, 'title'=>'title 2','link'=>'link 2','position'=>'0'],
                ['id'=>3, 'parent_id'=>1, 'title'=>'title 3','link'=>'link 3','position'=>'0'],
                ['id'=>4, 'parent_id'=>0, 'title'=>'title 4','link'=>'link 4','position'=>'0'],
            ];
    }

    /**
     * Build the HTML for the menu 
     */
    function get_menu_html( $root_id = 0 )
    {
        $this->html  = array();
        $this->items = $this->get_menu_items();

        foreach ( $this->items as $item )
            $children[$item['parent_id']][] = $item;

        // loop will be false if the root has no children (i.e., an empty menu!)
        $loop = !empty( $children[$root_id] );

        // initializing $parent as the root
        $parent = $root_id;
        $parent_stack = array();

        // HTML wrapper for the menu (open)
        $this->html[] = '<ul>';

        while ( $loop && ( ( $option = each( $children[$parent] ) ) || ( $parent > $root_id ) ) )
        {
            if ( $option === false )
            {
                $parent = array_pop( $parent_stack );

                // HTML for menu item containing childrens (close)
                $this->html[] = str_repeat( "\t", ( count( $parent_stack )   1 ) * 2 ) . '</ul>';
                $this->html[] = str_repeat( "\t", ( count( $parent_stack )   1 ) * 2 - 1 ) . '</li>';
            }
            elseif ( !empty( $children[$option['value']['id']] ) )
            {
                $tab = str_repeat( "\t", ( count( $parent_stack )   1 ) * 2 - 1 );

                // HTML for menu item containing childrens (open)
                $this->html[] = sprintf(
                    '%1$s<li><a href="%2$s">%3$s</a>',
                    $tab,   // %1$s = tabulation
                    $option['value']['link'],   // %2$s = link (URL)
                    $option['value']['title']   // %3$s = title
                ); 
                $this->html[] = $tab . "\t" . '<ul >';

                array_push( $parent_stack, $option['value']['parent_id'] );
                $parent = $option['value']['id'];
            }
            else
                // HTML for menu item with no children (aka "leaf") 
                $this->html[] = sprintf(
                    '%1$s<li><a href="%2$s">%3$s</a></li>',
                    str_repeat( "\t", ( count( $parent_stack )   1 ) * 2 - 1 ),   // %1$s = tabulation
                    $option['value']['link'],   // %2$s = link (URL)
                    $option['value']['title']   // %3$s = title
                );
        }

        // HTML wrapper for the menu (close)
        $this->html[] = '</ul>';

        return implode( "\r\n", $this->html );
    }
}

$menu = new MenuBuilder();
echo '<pre>' . $menu->get_menu_html() . '</pre>';



?>

It still has "each" in there, but it's isolated so it should be easier to convert over. Can play with it here: https://sandbox.onlinephpfunctions.com/

Working on getting this converted now...

Here's a drop-in replacement I wrote:

    public function each(&$ar)
    {
        $key = key($ar);
        if($key === null){return false;}
        $current = current($ar);
        
        try
        {
        return [$key,$current,'key'=>$key,'value'=>$current];
        }finally{
          next($ar);  
        };
        
    }

Then instead of:

while ( $loop && ( ( $option = each( $children[$parent] ) ) || ( $parent > $root_id ) ) )

use:

while ( $loop && ( ( $option = $this->each( $children[$parent] ) ) || ( $parent > $root_id ) ) )

i.e. using $this->each instead of each.

Final code:

<?php

/**
 * Generate HTML for multi-dimensional menu from MySQL database
 * with ONE QUERY and WITHOUT RECURSION 
 * @author J. Bruni
 */
class MenuBuilder
{

    /**
     * Menu items
     */
    var $items = array();

    /**
     * HTML contents
     */
    var $html  = array();


    public function each(&$ar)
    {
        $key = key($ar);
        if($key === null){return false;}
        $current = current($ar);
        
        try
        {
        return [$key,$current,'key'=>$key,'value'=>$current];
        }finally{
          next($ar);  
        };
        
    }


    /**
     * Get all menu items from database
     */
    function get_menu_items()
    {
        // Change the field names and the table name in the query below to match tour needs
        return [
                ['id'=>1, 'parent_id'=>0, 'title'=>'title 1','link'=>'link 1','position'=>'0'],
                ['id'=>2, 'parent_id'=>1, 'title'=>'title 2','link'=>'link 2','position'=>'0'],
                ['id'=>3, 'parent_id'=>1, 'title'=>'title 3','link'=>'link 3','position'=>'0'],
                ['id'=>4, 'parent_id'=>0, 'title'=>'title 4','link'=>'link 4','position'=>'0'],
            ];
    }

    /**
     * Build the HTML for the menu 
     */
    function get_menu_html( $root_id = 0 )
    {
        $this->html  = array();
        $this->items = $this->get_menu_items();

        foreach ( $this->items as $item )
            $children[$item['parent_id']][] = $item;

        // loop will be false if the root has no children (i.e., an empty menu!)
        $loop = !empty( $children[$root_id] );

        // initializing $parent as the root
        $parent = $root_id;
        $parent_stack = array();

        // HTML wrapper for the menu (open)
        $this->html[] = '<ul>';

        while ( $loop && ( ( $option = $this->each( $children[$parent] ) ) || ( $parent > $root_id ) ) )
        {
            if ( $option === false )
            {
                $parent = array_pop( $parent_stack );

                // HTML for menu item containing childrens (close)
                $this->html[] = str_repeat( "\t", ( count( $parent_stack )   1 ) * 2 ) . '</ul>';
                $this->html[] = str_repeat( "\t", ( count( $parent_stack )   1 ) * 2 - 1 ) . '</li>';
            }
            elseif ( !empty( $children[$option['value']['id']] ) )
            {
                $tab = str_repeat( "\t", ( count( $parent_stack )   1 ) * 2 - 1 );

                // HTML for menu item containing childrens (open)
                $this->html[] = sprintf(
                    '%1$s<li><a href="%2$s">%3$s</a>',
                    $tab,   // %1$s = tabulation
                    $option['value']['link'],   // %2$s = link (URL)
                    $option['value']['title']   // %3$s = title
                ); 
                $this->html[] = $tab . "\t" . '<ul >';

                array_push( $parent_stack, $option['value']['parent_id'] );
                $parent = $option['value']['id'];
            }
            else
                // HTML for menu item with no children (aka "leaf") 
                $this->html[] = sprintf(
                    '%1$s<li><a href="%2$s">%3$s</a></li>',
                    str_repeat( "\t", ( count( $parent_stack )   1 ) * 2 - 1 ),   // %1$s = tabulation
                    $option['value']['link'],   // %2$s = link (URL)
                    $option['value']['title']   // %3$s = title
                );
        }

        // HTML wrapper for the menu (close)
        $this->html[] = '</ul>';

        return implode( "\r\n", $this->html );
    }
}

$menu = new MenuBuilder();
echo '<pre>' . $menu->get_menu_html() . '</pre>';



?>

Tests fine over here: https://sandbox.onlinephpfunctions.com/