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.
silverstripe · Nov 30, 2012 · d12b497 · d12b497
1 parent fa2057b
commit d12b497
Show file tree

Hide file tree

Showing 2 changed files with 81 additions and 69 deletions.
diff --git 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 @@ 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);
 	}
 
 	/**
@@ -176,16 +181,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);
 	}
 
 	/**
@@ -473,7 +482,6 @@ public function exclude() {
 			}
 		}
 
-		$itemsToKeep = array();
 
 		$hitsRequiredToRemove = count($removeUs);
 		$matches = array();
@@ -488,13 +496,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;
 	}
 

diff --git a/tests/model/ArrayListTest.php b/tests/model/ArrayListTest.php
@@ -443,15 +443,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');
@@ -481,12 +481,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');
 	}
@@ -496,16 +496,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());
@@ -517,23 +517,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());
 	}
@@ -543,26 +543,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());
 	}
@@ -572,29 +572,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());
 	}