ARTICLE AD BOX
Yes, it is unsafe. Removing an element during a foreach loop will skip the very next element in the collection, creating a bug. this is not a theoretical risk but a guaranteed PHP behavior. and even If you remove elements while iterating directly over a Doctrine Collection, you will likely skip the element immediately following the one you removed.
for example Imagine you're checking people in a line for tickets:
Line: Person A → Person B → Person C
You check Person A → No ticket → Remove them
Line shifts: Person B (now first) → Person C (now second)
Your finger moves to the next position (now Person C)
You skipped Person B entirely!
That is exactly what happens in your code.
// WHY THIS WILL SKIP ELEMENTS - DO NOT USE foreach ($dossier->getEcheances() as $echeance) { if (!$this->hasActiveReglement($echeance)) { $dossier->removeEcheance($echeance); // <- This causes the skip } } //Internal Process: //getEcheances() returns a PersistentCollection //foreach triggers Doctrine to load all Echeance entities into a PHP array //When you remove an element, PHP's internal array shifts //The foreach pointer doesn't adjust → skips the next element // THIS IS CORRECT - USE THIS foreach ($dossier->getEcheances()->toArray() as $echeance) { if (!$this->hasActiveReglement($echeance)) { $dossier->removeEcheance($echeance); } } // you can also use Other Safe Patterns like (filter() and "collect first") // or you can simply Use filter() $activeEcheances = $dossier->getEcheances()->filter( fn($echeance) => $this->hasActiveReglement($echeance) ); $dossier->setEcheances($activeEcheances); // or Even for simply collect first , Remove After. $toRemove = []; foreach ($dossier->getEcheances() as $echeance) { if (!$this->hasActiveReglement($echeance)) { $toRemove[] = $echeance; } } foreach ($toRemove as $echeance) { $dossier->removeEcheance($echeance); }2. This is NOT just a precaution, The Elements are really skipped. It is a guaranteed behavior of PHP when you modify the array you are currently looping over.
3. It comes from PHP's foreach itself. This happens because Doctrine’s collection uses a standard PHP array internally, and PHP’s foreach doesn’t adjust its internal pointer when the array changes. Doctrine's PersistentCollection uses a standard PHP array internally to store items. When foreach starts, PHP creates an internal pointer. If you remove() an item, the internal array changes, but PHP's pointer doesn't adjust, causing the skip. Doctrine doesn't cause this; it inherits the behavior from PHP.
4. Yes , toArray() is the standard and recommended solution. It's a simple, one-word fix that guarantees safety.
Please Note:
Never modify a Doctrine\Common\Collections\Collection during a foreach loop. PHP's internal pointer will skip the element following any removal (PHP foreach behavior).
Always use ->toArray() on the collection first. This creates a stable snapshot for safe iteration (Doctrine Collections API).
Doctrine inherits this from PHP. Its PersistentCollection uses a standard PHP array internally, so the standard PHP rules apply (Doctrine Working with Collections).
