2

I have an array of objects which have id and parentId attributes. The id value is unique, but more than one object may have the same parentId value.

If more than one object has the same parentId value, I want to remove all but one (i.e. remove the "siblings").

I thought I could do this easily with nested foreach loops, but it's not working as I expected.

Here's an example:

$objArray = [];

for($i = 0; $i < 2; $i++) {
    $obj = new stdClass();
    $obj->id = $i;
    $obj->parentId = 1;

    $objArray[] = $obj;
}

for($i = 2; $i < 4; $i++) {
    $obj = new stdClass();
    $obj->id = $i;
    $obj->parentId = 2;

    $objArray[] = $obj;
}

echo 'Before unsetting siblings:<pre>';
print_r($objArray);
echo '</pre>';

// loop over $objArray and remove elements with the same ->parentId (leaving one)
foreach ($objArray as $keyOuter => $objOuter) {
    foreach ($objArray as $keyInner => $objInner) {
        if ($objInner->id != $objOuter->id             // if the inner object is NOT the same as the outer object (i.e. it's a different object)
         && $objInner->parentId == $objOuter->parentId // and if the parent IDs are the same
        ) {
            unset($objArray[$keyInner]); // unset the inner object
        }
    }
}

echo 'After unsetting siblings:<pre>';
print_r($objArray);
echo '</pre>';

Output:

Before unsetting siblings:

Array
(
    [0] => stdClass Object
        (
            [id] => 0
            [parentId] => 1
        )

    [1] => stdClass Object
        (
            [id] => 1
            [parentId] => 1
        )

    [2] => stdClass Object
        (
            [id] => 2
            [parentId] => 2
        )

    [3] => stdClass Object
        (
            [id] => 3
            [parentId] => 2
        )

)

After unsetting siblings:

Array
(
)

I expected the first and third objects in the array to remain after the foreach loops, but as you can see all of the objects in the array are deleted.

What am I missing here?

6
  • The documentation states that modifying an array while being looped over with a foreach leads to undefined behaviour (the foreach loop uses a pointer internal to the array, the same the one used with function like next, current, etc.). I'm even wondering if 2 nested loops should actually work given the explanation. Commented Feb 19, 2015 at 2:14
  • 1
    @didierc I reread the documentation and do not see anything on what you said. Commented Feb 19, 2015 at 2:18
  • You are correct. The issue is that one should not modify the internal pointer, I wrongly extrapolated to any modification to the array. Here's the quote: As foreach relies on the internal array pointer, changing it within the loop may lead to unexpected Behavior. Commented Feb 19, 2015 at 2:21
  • Note that still, using a foreach within a foreach should have the outer loop be disturbed by the inner loop resetting the internal pointer. Commented Feb 19, 2015 at 2:22
  • @didierc I see nested foreach loops in PHP code all the time, though? Commented Feb 19, 2015 at 2:27

3 Answers 3

1

I added this line in your inner loop:

echo 'Matched id #'.$objOuter->id.' parent #'.$objOuter->parentId.' with id #'.$objInner->id.' parent #'.$objInner->parentId."\r\n";

Which yielded:

Matched id #0 parent #1 with id #1 parent #1

Matched id #1 parent #1 with id #0 parent #1

Matched id #2 parent #2 with id #3 parent #2

Matched id #3 parent #2 with id #2 parent #2

Another demonstration (!! indicates no match/deletion, == indicates a parent match with deletion):

0/1!!0/1 0/1==1/1 0/1!!2/2 0/1!!3/2 
1/1==0/1 1/1!!2/2 1/1!!3/2 
2/2!!2/2 2/2==3/2 
3/2==2/2 

See the pattern? While your inner loop is getting the latest version of $objArray after you have unset the element that has been matched, your outer loop doesn't have the new version because foreach actually keeps a temporary cloned array for the $objOuter and $keyOuter values.

Here's a proof of concept:

$array = array(1,2,3,4);

foreach ($array as $a) {
    if (isset($array)) unset($array);
    echo $a;
}

Output:

1234

Or this:

$array = array(1,2,3,4);

foreach ($array as $a) {
    $array[3] = 100;
    echo $a; // Output is still 1234
}

Or this:

$array = array(1,2,3,4);

foreach ($array as $a) {
    if (isset($array[3])) unset($array[3]);
    echo $a; // Yet again 1234
}

If $array doesn't exist anymore, why does the loop still keep going? By the same logic, why isn't the output of my second example 123100? The same flaw/bug exists with your outer loop.

I'd rather create a new filtered array instead of trying to delete from the original with nested loops:

$newArray = array();
foreach ($objArray as $obj) {
    if (!isset($newArray[$obj->parentId])) { // Use the index to test for existing parent IDs
        $newArray[$obj->parentId] = $obj;
    }
}
// Optional - use array_values to get rid of parentId in the indices
$newArray = array_values($newArray);
// or you can just do this if you want to replace $objArray
$objArray = array_values($newArray);
unset($newArray);

You can also keep another array of existing parent keys and then delete from the existing array if the key already exists:

$existingParents = array();
foreach ($objArray as $key => $obj) {
    if (isset($existingParents[$obj->parentId])) {
        unset($objArray[$key]);
    } else {
        $existingParents[$obj->parentId] = true;
    }
}
Sign up to request clarification or add additional context in comments.

3 Comments

What do you mean by "your loop is too liberal"?
@Nate Gave some clarification. Try my proof of concept code for yourself to see what I mean.
I really like your updated solution. That's probably much more efficient than nested loops.
1

The first two lines of your for loops are the same, use var_dump to make sure for yourself:

foreach ($objArray as $keyOuter => $objOuter) {
    foreach ($objArray as $keyInner => $objInner) { 
        var_dump($objArray, $objInner); //the same object is returned
        ...

so technically the objOuter will always equal objInner in the first run of the second foreach, hence deleting the object, at the end you will have deleted all objects.

for this piece of code give your variable a different name:

for($i = 2; $i < 4; $i++) {
    $obj = new stdClass();
    $obj->id = $i;
    $obj->parentId = 2;

    $objArray2[] = $obj; //instead of $objArray
}

This is the final code and it works the way you wanted it to:

for($i = 0; $i < 2; $i++) {
    $obj = new stdClass();
    $obj->id = $i;
    $obj->parentId = 1;

    $objArray[] = $obj;
}

for($i = 2; $i < 4; $i++) {
    $obj = new stdClass();
    $obj->id = $i;
    $obj->parentId = 2;

    $objArray2[] = $obj;
}

echo 'Before unsetting siblings:<pre>';
print_r($objArray);
echo '</pre>';

foreach ($objArray as $keyOuter => $objOuter) {
    foreach ($objArray2 as $keyInner => $objInner) {
        if ($objInner->id != $objOuter->id 
            && $objInner->parentId == $objOuter->parentId) {
        unset($objArray[$keyInner]); // unset the inner object
        }
    }
}

echo 'After unsetting siblings:<pre>';
print_r($objArray);
echo '</pre>';

Comments

1

Quick solution:

$objArray = [];

for($i = 0; $i < 2; $i++) {
    $obj = new stdClass();
    $obj->id = $i;
    $obj->parentId = 1;

    $objArray[] = $obj;
}

for($i = 2; $i < 4; $i++) {
    $obj = new stdClass();
    $obj->id = $i;
    $obj->parentId = 2;

    $objArray[] = $obj;
}

echo 'Before unsetting siblings:<pre>';
print_r($objArray);
echo '</pre>';

// loop over $objArray and remove elements with the same ->parentId (leaving one)
$max = count($objArray);
for ($i=0; $i<$max; $i++) {
    $objOuter = $objArray[$i];
    foreach ($objArray as $i2=>$objInner) {
        if ($objInner->id != $objOuter->id             // if the inner object is NOT the same as the outer object (i.e. it's a different object)
         && $objInner->parentId == $objOuter->parentId // and if the parent IDs are the same
        ) {
            unset($objArray[$i2]);
            $max=$max-1;// unset the inner object
        }
    }
}

echo 'After unsetting siblings:<pre>';
print_r($objArray);
echo '</pre>';

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.