Skip to content
Browse files

BUGFIX: ArrayList now discards keys of the array passed in and keeps …

…the numerically indexed array sequential.

This fixes FirstLast and EvenOdd in templates, and makes ArrayList more consistent, as several methods already discarded the keys.
  • Loading branch information...
1 parent 8f27e7a commit b6017a7c902e5e66835dfbd4884924a56ac9a370 @ajoneil ajoneil committed with halkyon
Showing with 81 additions and 69 deletions.
  1. +19 −7 model/ArrayList.php
  2. +62 −62 tests/model/ArrayListTest.php
View
26 model/ArrayList.php
@@ -19,7 +19,7 @@ class ArrayList extends ViewableData implements SS_List, SS_Filterable, SS_Sorta
* @param array $items - an initial array to fill this object with
*/
public function __construct(array $items = array()) {
- $this->items = $items;
+ $this->items = array_values($items);
parent::__construct();
}
@@ -138,9 +138,14 @@ public function add($item) {
* @param mixed $item
*/
public function remove($item) {
+ $renumberKeys = false;
foreach ($this->items as $key => $value) {
- if ($item === $value) unset($this->items[$key]);
+ if ($item === $value) {
+ $renumberKeys = true;
+ unset($this->items[$key]);
+ }
}
+ if($renumberKeys) $this->items = array_values($this->items);
}
/**
@@ -177,16 +182,20 @@ public function merge($with) {
*/
public function removeDuplicates($field = 'ID') {
$seen = array();
+ $renumberKeys = false;
foreach ($this->items as $key => $item) {
$value = $this->extractValue($item, $field);
if (array_key_exists($value, $seen)) {
+ $renumberKeys = true;
unset($this->items[$key]);
}
$seen[$value] = true;
}
+
+ if($renumberKeys) $this->items = array_values($this->items);
}
/**
@@ -474,7 +483,6 @@ public function exclude() {
}
}
- $itemsToKeep = array();
$hitsRequiredToRemove = count($removeUs);
$matches = array();
@@ -489,13 +497,17 @@ public function exclude() {
}
$keysToRemove = array_keys($matches,$hitsRequiredToRemove);
- // TODO 3.1: This currently mutates existing array
- $list = /* clone */ $this;
- foreach($keysToRemove as $itemToRemoveIdx){
- $list->remove($this->items[$itemToRemoveIdx]);
+ $itemsToKeep = array();
+ foreach($this->items as $key => $value) {
+ if(!in_array($key, $keysToRemove)) {
+ $itemsToKeep[] = $value;
+ }
}
+ // TODO 3.1: This currently mutates existing array
+ $list = /* clone */ $this;
+ $list->items = $itemsToKeep;
return $list;
}
View
124 tests/model/ArrayListTest.php
@@ -428,15 +428,15 @@ public function testMultipleWithArrayFilterAdvanced() {
*/
public function testSimpleExclude() {
$list = new ArrayList(array(
- 0=>array('Name' => 'Steve'),
- 1=>array('Name' => 'Bob'),
- 2=>array('Name' => 'John')
+ array('Name' => 'Steve'),
+ array('Name' => 'Bob'),
+ array('Name' => 'John')
));
$list->exclude('Name', 'Bob');
$expected = array(
- 0=>array('Name' => 'Steve'),
- 2=>array('Name' => 'John')
+ array('Name' => 'Steve'),
+ array('Name' => 'John')
);
$this->assertEquals(2, $list->count());
$this->assertEquals($expected, $list->toArray(), 'List should not contain Bob');
@@ -466,12 +466,12 @@ public function testSimpleExcludeNoMatch() {
*/
public function testSimpleExcludeWithArray() {
$list = new ArrayList(array(
- 0=>array('Name' => 'Steve'),
- 1=>array('Name' => 'Bob'),
- 2=>array('Name' => 'John')
+ array('Name' => 'Steve'),
+ array('Name' => 'Bob'),
+ array('Name' => 'John')
));
$list->exclude('Name', array('Steve','John'));
- $expected = array(1=>array('Name' => 'Bob'));
+ $expected = array(array('Name' => 'Bob'));
$this->assertEquals(1, $list->count());
$this->assertEquals($expected, $list->toArray(), 'List should only contain Bob');
}
@@ -481,16 +481,16 @@ public function testSimpleExcludeWithArray() {
*/
public function testExcludeWithTwoArrays() {
$list = new ArrayList(array(
- 0=>array('Name' => 'Bob' , 'Age' => 21),
- 1=>array('Name' => 'Bob' , 'Age' => 32),
- 2=>array('Name' => 'John', 'Age' => 21)
+ array('Name' => 'Bob' , 'Age' => 21),
+ array('Name' => 'Bob' , 'Age' => 32),
+ array('Name' => 'John', 'Age' => 21)
));
$list->exclude(array('Name' => 'Bob', 'Age' => 21));
$expected = array(
- 1=>array('Name' => 'Bob', 'Age' => 32),
- 2=>array('Name' => 'John', 'Age' => 21)
+ array('Name' => 'Bob', 'Age' => 32),
+ array('Name' => 'John', 'Age' => 21)
);
$this->assertEquals(2, $list->count());
@@ -502,23 +502,23 @@ public function testExcludeWithTwoArrays() {
*/
public function testMultipleExclude() {
$list = new ArrayList(array(
- 0 => array('Name' => 'bob', 'Age' => 10),
- 1 => array('Name' => 'phil', 'Age' => 11),
- 2 => array('Name' => 'bob', 'Age' => 12),
- 3 => array('Name' => 'phil', 'Age' => 12),
- 4 => array('Name' => 'bob', 'Age' => 14),
- 5 => array('Name' => 'phil', 'Age' => 14),
- 6 => array('Name' => 'bob', 'Age' => 16),
- 7 => array('Name' => 'phil', 'Age' => 16)
+ array('Name' => 'bob', 'Age' => 10),
+ array('Name' => 'phil', 'Age' => 11),
+ array('Name' => 'bob', 'Age' => 12),
+ array('Name' => 'phil', 'Age' => 12),
+ array('Name' => 'bob', 'Age' => 14),
+ array('Name' => 'phil', 'Age' => 14),
+ array('Name' => 'bob', 'Age' => 16),
+ array('Name' => 'phil', 'Age' => 16)
));
$list->exclude(array('Name'=>array('bob','phil'),'Age'=>array(10, 16)));
$expected = array(
- 1 => array('Name' => 'phil', 'Age' => 11),
- 2 => array('Name' => 'bob', 'Age' => 12),
- 3 => array('Name' => 'phil', 'Age' => 12),
- 4 => array('Name' => 'bob', 'Age' => 14),
- 5 => array('Name' => 'phil', 'Age' => 14),
+ array('Name' => 'phil', 'Age' => 11),
+ array('Name' => 'bob', 'Age' => 12),
+ array('Name' => 'phil', 'Age' => 12),
+ array('Name' => 'bob', 'Age' => 14),
+ array('Name' => 'phil', 'Age' => 14),
);
$this->assertEquals($expected, $list->toArray());
}
@@ -528,26 +528,26 @@ public function testMultipleExclude() {
*/
public function testMultipleExcludeNoMatch() {
$list = new ArrayList(array(
- 0 => array('Name' => 'bob', 'Age' => 10),
- 1 => array('Name' => 'phil', 'Age' => 11),
- 2 => array('Name' => 'bob', 'Age' => 12),
- 3 => array('Name' => 'phil', 'Age' => 12),
- 4 => array('Name' => 'bob', 'Age' => 14),
- 5 => array('Name' => 'phil', 'Age' => 14),
- 6 => array('Name' => 'bob', 'Age' => 16),
- 7 => array('Name' => 'phil', 'Age' => 16)
+ array('Name' => 'bob', 'Age' => 10),
+ array('Name' => 'phil', 'Age' => 11),
+ array('Name' => 'bob', 'Age' => 12),
+ array('Name' => 'phil', 'Age' => 12),
+ array('Name' => 'bob', 'Age' => 14),
+ array('Name' => 'phil', 'Age' => 14),
+ array('Name' => 'bob', 'Age' => 16),
+ array('Name' => 'phil', 'Age' => 16)
));
$list->exclude(array('Name'=>array('bob','phil'),'Age'=>array(10, 16),'Bananas'=>true));
$expected = array(
- 0 => array('Name' => 'bob', 'Age' => 10),
- 1 => array('Name' => 'phil', 'Age' => 11),
- 2 => array('Name' => 'bob', 'Age' => 12),
- 3 => array('Name' => 'phil', 'Age' => 12),
- 4 => array('Name' => 'bob', 'Age' => 14),
- 5 => array('Name' => 'phil', 'Age' => 14),
- 6 => array('Name' => 'bob', 'Age' => 16),
- 7 => array('Name' => 'phil', 'Age' => 16)
+ array('Name' => 'bob', 'Age' => 10),
+ array('Name' => 'phil', 'Age' => 11),
+ array('Name' => 'bob', 'Age' => 12),
+ array('Name' => 'phil', 'Age' => 12),
+ array('Name' => 'bob', 'Age' => 14),
+ array('Name' => 'phil', 'Age' => 14),
+ array('Name' => 'bob', 'Age' => 16),
+ array('Name' => 'phil', 'Age' => 16)
);
$this->assertEquals($expected, $list->toArray());
}
@@ -557,29 +557,29 @@ public function testMultipleExcludeNoMatch() {
*/
public function testMultipleExcludeThreeArguments() {
$list = new ArrayList(array(
- 0 => array('Name' => 'bob', 'Age' => 10, 'HasBananas'=>false),
- 1 => array('Name' => 'phil','Age' => 11, 'HasBananas'=>true),
- 2 => array('Name' => 'bob', 'Age' => 12, 'HasBananas'=>true),
- 3 => array('Name' => 'phil','Age' => 12, 'HasBananas'=>true),
- 4 => array('Name' => 'bob', 'Age' => 14, 'HasBananas'=>false),
- 4 => array('Name' => 'ann', 'Age' => 14, 'HasBananas'=>true),
- 5 => array('Name' => 'phil','Age' => 14, 'HasBananas'=>false),
- 6 => array('Name' => 'bob', 'Age' => 16, 'HasBananas'=>false),
- 7 => array('Name' => 'phil','Age' => 16, 'HasBananas'=>true),
- 8 => array('Name' => 'clair','Age' => 16, 'HasBananas'=>true)
+ array('Name' => 'bob', 'Age' => 10, 'HasBananas'=>false),
+ array('Name' => 'phil','Age' => 11, 'HasBananas'=>true),
+ array('Name' => 'bob', 'Age' => 12, 'HasBananas'=>true),
+ array('Name' => 'phil','Age' => 12, 'HasBananas'=>true),
+ array('Name' => 'bob', 'Age' => 14, 'HasBananas'=>false),
+ array('Name' => 'ann', 'Age' => 14, 'HasBananas'=>true),
+ array('Name' => 'phil','Age' => 14, 'HasBananas'=>false),
+ array('Name' => 'bob', 'Age' => 16, 'HasBananas'=>false),
+ array('Name' => 'phil','Age' => 16, 'HasBananas'=>true),
+ array('Name' => 'clair','Age' => 16, 'HasBananas'=>true)
));
$list->exclude(array('Name'=>array('bob','phil'),'Age'=>array(10, 16),'HasBananas'=>true));
$expected = array(
- 0 => array('Name' => 'bob', 'Age' => 10, 'HasBananas'=>false),
- 1 => array('Name' => 'phil','Age' => 11, 'HasBananas'=>true),
- 2 => array('Name' => 'bob', 'Age' => 12, 'HasBananas'=>true),
- 3 => array('Name' => 'phil','Age' => 12, 'HasBananas'=>true),
- 4 => array('Name' => 'bob', 'Age' => 14, 'HasBananas'=>false),
- 4 => array('Name' => 'ann', 'Age' => 14, 'HasBananas'=>true),
- 5 => array('Name' => 'phil','Age' => 14, 'HasBananas'=>false),
- 6 => array('Name' => 'bob', 'Age' => 16, 'HasBananas'=>false),
- 8 => array('Name' => 'clair','Age' => 16, 'HasBananas'=>true)
+ array('Name' => 'bob', 'Age' => 10, 'HasBananas'=>false),
+ array('Name' => 'phil','Age' => 11, 'HasBananas'=>true),
+ array('Name' => 'bob', 'Age' => 12, 'HasBananas'=>true),
+ array('Name' => 'phil','Age' => 12, 'HasBananas'=>true),
+ array('Name' => 'bob', 'Age' => 14, 'HasBananas'=>false),
+ array('Name' => 'ann', 'Age' => 14, 'HasBananas'=>true),
+ array('Name' => 'phil','Age' => 14, 'HasBananas'=>false),
+ array('Name' => 'bob', 'Age' => 16, 'HasBananas'=>false),
+ array('Name' => 'clair','Age' => 16, 'HasBananas'=>true)
);
$this->assertEquals($expected, $list->toArray());
}

0 comments on commit b6017a7

Please sign in to comment.
Something went wrong with that request. Please try again.