ArrayCollection::remove is unable to remove NULL items #135

Closed
MikeParkin opened this Issue Apr 13, 2012 · 1 comment

Comments

Projects
None yet
2 participants

I got caught out today with an ArrayCollection that has a null value inside. Because internally the remove method of ArrayCollection uses isset on the internal $elements array;

Code to replicate

$collection = new ArrayCollection();

$collection->add(null);
print_r($collection);

$collection->remove(0);
print_r($collection);

Actual Output

Doctrine\Common\Collections\ArrayCollection Object
(
    [_elements:Doctrine\Common\Collections\ArrayCollection:private] => Array
    (
        [0] =>
    )

)
Doctrine\Common\Collections\ArrayCollection Object
(
    [_elements:Doctrine\Common\Collections\ArrayCollection:private] => Array
    (
        [0] =>
    )
)

Expected Output

 Doctrine\Common\Collections\ArrayCollection Object
(
    [_elements:Doctrine\Common\Collections\ArrayCollection:private] => Array
    (
        [0] =>
    )

)
Doctrine\Common\Collections\ArrayCollection Object
(
    [_elements:Doctrine\Common\Collections\ArrayCollection:private] => Array
    (
    )
)

Fix

The fix for this is quite easy, just swap the isset() call in remove to use array_key_exists:

public function remove($key)
{
    if(array_key_exists($key, $this->_elements)) {
        $removed = $this->_elements[$key];
        unset($this->_elements[$key]);

        return $removed;
    }

    return null;
}

I was wondering if this is a known issue or whether or not it was designed to work this way? To me it seems very abnormal that you can add a null value, but you cannot remove it, but maybe there is something I am missing. As a work around I could do the following as it did not matter to me about which null element was removed.

$collection->removeElement(null);

@Ocramius Ocramius added a commit to Ocramius/common that referenced this issue Nov 9, 2012

@Ocramius Ocramius Adding failing test for doctrine/common#135 5c5c74b

@Ocramius Ocramius added a commit to Ocramius/common that referenced this issue Nov 9, 2012

@Ocramius Ocramius Providing fix for doctrine/common#135 df4e23e

@beberlei beberlei pushed a commit to doctrine/collections that referenced this issue Jan 12, 2013

@Ocramius Ocramius Adding failing test for doctrine/common#135 3d53e5c

@beberlei beberlei pushed a commit to doctrine/collections that referenced this issue Jan 12, 2013

@Ocramius Ocramius Providing fix for doctrine/common#135 6b64553
Owner

Ocramius commented Dec 23, 2013

This was already fixed

Ocramius closed this Dec 23, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment