From 9d74c99e085f6940225fae959746930a6ceadc28 Mon Sep 17 00:00:00 2001 From: Andrew O'Neil Date: Mon, 12 Nov 2012 16:25:55 +1300 Subject: [PATCH] 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. --- model/ArrayList.php | 26 +++++-- tests/model/ArrayListTest.php | 124 +++++++++++++++++----------------- 2 files changed, 81 insertions(+), 69 deletions(-) diff --git a/model/ArrayList.php b/model/ArrayList.php index b60d028aa..dccd6b3e2 100644 --- a/model/ArrayList.php +++ b/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(); } @@ -137,9 +137,14 @@ class ArrayList extends ViewableData implements SS_List, SS_Filterable, SS_Sorta * @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); } /** @@ -176,16 +181,20 @@ class ArrayList extends ViewableData implements SS_List, SS_Filterable, SS_Sorta */ 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); } /** @@ -473,7 +482,6 @@ class ArrayList extends ViewableData implements SS_List, SS_Filterable, SS_Sorta } } - $itemsToKeep = array(); $hitsRequiredToRemove = count($removeUs); $matches = array(); @@ -488,13 +496,17 @@ class ArrayList extends ViewableData implements SS_List, SS_Filterable, SS_Sorta } $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; } diff --git a/tests/model/ArrayListTest.php b/tests/model/ArrayListTest.php index 3087d2db0..0d568f05d 100644 --- a/tests/model/ArrayListTest.php +++ b/tests/model/ArrayListTest.php @@ -443,15 +443,15 @@ class ArrayListTest extends SapphireTest { */ 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'); @@ -481,12 +481,12 @@ class ArrayListTest extends SapphireTest { */ 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'); } @@ -496,16 +496,16 @@ class ArrayListTest extends SapphireTest { */ 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()); @@ -517,23 +517,23 @@ class ArrayListTest extends SapphireTest { */ 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()); } @@ -543,26 +543,26 @@ class ArrayListTest extends SapphireTest { */ 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()); } @@ -572,29 +572,29 @@ class ArrayListTest extends SapphireTest { */ 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()); }