Fix some issues of lithium\util\Collection. #474

Merged
merged 3 commits into from May 23, 2012

2 participants

@jails
Union of RAD member

Solving some bugs on unset() and changing the ArrayAccess::offsetExist() behavior. ArrayAccess::offsetExist() should indicate if an offset exists and imo it would work as array_key_exists(). ArrayAccess::offsetExist() is indeed a magic method called on isset() but imo it's an inconsistency in the PHP language.

@nateabele nateabele and 1 other commented on an outdated diff May 23, 2012
tests/cases/util/CollectionTest.php
$data = array(
- 'Hello',
@nateabele
Union of RAD member

I don't follow why you changed the test here. Wouldn't it be better to have both?

@jails
Union of RAD member
jails added a note May 23, 2012

Yeah a bug occurs when you try to unset the first element in a foreach but you're right, it's better to keep both test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jails
Union of RAD member

I refactored the tests to be more consistent with older tests

@nateabele nateabele commented on the diff May 23, 2012
util/Collection.php
@@ -32,7 +32,7 @@
*
* $coll = new Collection(array('data' => array(0, 1, 2, 3, 4)));
*
- * $coll->first(); // 1 (the first non-empty value)
@nateabele
Union of RAD member

Just to clarify, the change here points out a BC break in first()?

@jails
Union of RAD member
jails added a note May 23, 2012

Well I just found that it works like that so i decide to update the docblock but it's not a direct consequence of this PR. The BC break must have happened in a previous PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@nateabele nateabele merged commit 08d94f4 into UnionOfRAD:data May 23, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment