Skip to content
This repository
Browse code

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...
commit 9d74c99e085f6940225fae959746930a6ceadc28 1 parent 0fd6d14
Andrew O'Neil authored November 12, 2012 chillu committed December 11, 2012
26  model/ArrayList.php
@@ -19,7 +19,7 @@ class ArrayList extends ViewableData implements SS_List, SS_Filterable, SS_Sorta
19 19
 	 * @param array $items - an initial array to fill this object with
20 20
 	 */
21 21
 	public function __construct(array $items = array()) {
22  
-		$this->items = $items;
  22
+		$this->items = array_values($items);
23 23
 		parent::__construct();
24 24
 	}
25 25
 	
@@ -137,9 +137,14 @@ public function add($item) {
137 137
 	 * @param mixed $item 
138 138
 	 */
139 139
 	public function remove($item) {
  140
+		$renumberKeys = false;
140 141
 		foreach ($this->items as $key => $value) {
141  
-			if ($item === $value) unset($this->items[$key]);
  142
+			if ($item === $value) {
  143
+				$renumberKeys = true;
  144
+				unset($this->items[$key]);
  145
+			}
142 146
 		}
  147
+		if($renumberKeys) $this->items = array_values($this->items);
143 148
 	}
144 149
 
145 150
 	/**
@@ -176,16 +181,20 @@ public function merge($with) {
176 181
 	 */
177 182
 	public function removeDuplicates($field = 'ID') {
178 183
 		$seen = array();
  184
+		$renumberKeys = false;
179 185
 
180 186
 		foreach ($this->items as $key => $item) {
181 187
 			$value = $this->extractValue($item, $field);
182 188
 
183 189
 			if (array_key_exists($value, $seen)) {
  190
+				$renumberKeys = true;
184 191
 				unset($this->items[$key]);
185 192
 			}
186 193
 
187 194
 			$seen[$value] = true;
188 195
 		}
  196
+
  197
+		if($renumberKeys) $this->items = array_values($this->items);
189 198
 	}
190 199
 
191 200
 	/**
@@ -473,7 +482,6 @@ public function exclude() {
473 482
 			}
474 483
 		}
475 484
 
476  
-		$itemsToKeep = array();
477 485
 
478 486
 		$hitsRequiredToRemove = count($removeUs);
479 487
 		$matches = array();
@@ -488,13 +496,17 @@ public function exclude() {
488 496
 		}
489 497
 
490 498
 		$keysToRemove = array_keys($matches,$hitsRequiredToRemove);
491  
-		// TODO 3.1: This currently mutates existing array
492  
-		$list = /* clone */ $this;
493 499
 
494  
-		foreach($keysToRemove as $itemToRemoveIdx){
495  
-			$list->remove($this->items[$itemToRemoveIdx]);
  500
+		$itemsToKeep = array();
  501
+		foreach($this->items as $key => $value) {
  502
+			if(!in_array($key, $keysToRemove)) {
  503
+				$itemsToKeep[] = $value;
  504
+			}
496 505
 		}
497 506
 
  507
+		// TODO 3.1: This currently mutates existing array
  508
+		$list = /* clone */ $this;
  509
+		$list->items = $itemsToKeep;
498 510
 		return $list;
499 511
 	}
500 512
 
124  tests/model/ArrayListTest.php
@@ -443,15 +443,15 @@ public function testMultipleWithArrayFilterAdvanced() {
443 443
 	 */
444 444
 	public function testSimpleExclude() {
445 445
 		$list = new ArrayList(array(
446  
-			0=>array('Name' => 'Steve'),
447  
-			1=>array('Name' => 'Bob'),
448  
-			2=>array('Name' => 'John')
  446
+			array('Name' => 'Steve'),
  447
+			array('Name' => 'Bob'),
  448
+			array('Name' => 'John')
449 449
 		));
450 450
 		
451 451
 		$list->exclude('Name', 'Bob');
452 452
 		$expected = array(
453  
-			0=>array('Name' => 'Steve'),
454  
-			2=>array('Name' => 'John')
  453
+			array('Name' => 'Steve'),
  454
+			array('Name' => 'John')
455 455
 		);
456 456
 		$this->assertEquals(2, $list->count());
457 457
 		$this->assertEquals($expected, $list->toArray(), 'List should not contain Bob');
@@ -481,12 +481,12 @@ public function testSimpleExcludeNoMatch() {
481 481
 	 */
482 482
 	public function testSimpleExcludeWithArray() {
483 483
 		$list = new ArrayList(array(
484  
-			0=>array('Name' => 'Steve'),
485  
-			1=>array('Name' => 'Bob'),
486  
-			2=>array('Name' => 'John')
  484
+			array('Name' => 'Steve'),
  485
+			array('Name' => 'Bob'),
  486
+			array('Name' => 'John')
487 487
 		));
488 488
 		$list->exclude('Name', array('Steve','John'));
489  
-		$expected = array(1=>array('Name' => 'Bob'));
  489
+		$expected = array(array('Name' => 'Bob'));
490 490
 		$this->assertEquals(1, $list->count());
491 491
 		$this->assertEquals($expected, $list->toArray(), 'List should only contain Bob');
492 492
 	}
@@ -496,16 +496,16 @@ public function testSimpleExcludeWithArray() {
496 496
 	 */
497 497
 	public function testExcludeWithTwoArrays() {
498 498
 		$list = new ArrayList(array(
499  
-			0=>array('Name' => 'Bob' , 'Age' => 21),
500  
-			1=>array('Name' => 'Bob' , 'Age' => 32),
501  
-			2=>array('Name' => 'John', 'Age' => 21)
  499
+			array('Name' => 'Bob' , 'Age' => 21),
  500
+			array('Name' => 'Bob' , 'Age' => 32),
  501
+			array('Name' => 'John', 'Age' => 21)
502 502
 		));
503 503
 		
504 504
 		$list->exclude(array('Name' => 'Bob', 'Age' => 21));
505 505
 		
506 506
 		$expected = array(
507  
-			1=>array('Name' => 'Bob', 'Age' => 32),
508  
-			2=>array('Name' => 'John', 'Age' => 21)
  507
+			array('Name' => 'Bob', 'Age' => 32),
  508
+			array('Name' => 'John', 'Age' => 21)
509 509
 		);
510 510
 		
511 511
 		$this->assertEquals(2, $list->count());
@@ -517,23 +517,23 @@ public function testExcludeWithTwoArrays() {
517 517
 	 */
518 518
 	public function testMultipleExclude() {
519 519
 		$list = new ArrayList(array(
520  
-			0 => array('Name' => 'bob', 'Age' => 10),
521  
-			1 => array('Name' => 'phil', 'Age' => 11),
522  
-			2 => array('Name' => 'bob', 'Age' => 12),
523  
-			3 => array('Name' => 'phil', 'Age' => 12),
524  
-			4 => array('Name' => 'bob', 'Age' => 14),
525  
-			5 => array('Name' => 'phil', 'Age' => 14),
526  
-			6 => array('Name' => 'bob', 'Age' => 16),
527  
-			7 => array('Name' => 'phil', 'Age' => 16)
  520
+			array('Name' => 'bob', 'Age' => 10),
  521
+			array('Name' => 'phil', 'Age' => 11),
  522
+			array('Name' => 'bob', 'Age' => 12),
  523
+			array('Name' => 'phil', 'Age' => 12),
  524
+			array('Name' => 'bob', 'Age' => 14),
  525
+			array('Name' => 'phil', 'Age' => 14),
  526
+			array('Name' => 'bob', 'Age' => 16),
  527
+			array('Name' => 'phil', 'Age' => 16)
528 528
 		));
529 529
 
530 530
 		$list->exclude(array('Name'=>array('bob','phil'),'Age'=>array(10, 16)));
531 531
 		$expected = array(
532  
-			1 => array('Name' => 'phil', 'Age' => 11),
533  
-			2 => array('Name' => 'bob', 'Age' => 12),
534  
-			3 => array('Name' => 'phil', 'Age' => 12),
535  
-			4 => array('Name' => 'bob', 'Age' => 14),
536  
-			5 => array('Name' => 'phil', 'Age' => 14),
  532
+			array('Name' => 'phil', 'Age' => 11),
  533
+			array('Name' => 'bob', 'Age' => 12),
  534
+			array('Name' => 'phil', 'Age' => 12),
  535
+			array('Name' => 'bob', 'Age' => 14),
  536
+			array('Name' => 'phil', 'Age' => 14),
537 537
 		);
538 538
 		$this->assertEquals($expected, $list->toArray());
539 539
 	}
@@ -543,26 +543,26 @@ public function testMultipleExclude() {
543 543
 	 */
544 544
 	public function testMultipleExcludeNoMatch() {
545 545
 		$list = new ArrayList(array(
546  
-			0 => array('Name' => 'bob', 'Age' => 10),
547  
-			1 => array('Name' => 'phil', 'Age' => 11),
548  
-			2 => array('Name' => 'bob', 'Age' => 12),
549  
-			3 => array('Name' => 'phil', 'Age' => 12),
550  
-			4 => array('Name' => 'bob', 'Age' => 14),
551  
-			5 => array('Name' => 'phil', 'Age' => 14),
552  
-			6 => array('Name' => 'bob', 'Age' => 16),
553  
-			7 => array('Name' => 'phil', 'Age' => 16)
  546
+			array('Name' => 'bob', 'Age' => 10),
  547
+			array('Name' => 'phil', 'Age' => 11),
  548
+			array('Name' => 'bob', 'Age' => 12),
  549
+			array('Name' => 'phil', 'Age' => 12),
  550
+			array('Name' => 'bob', 'Age' => 14),
  551
+			array('Name' => 'phil', 'Age' => 14),
  552
+			array('Name' => 'bob', 'Age' => 16),
  553
+			array('Name' => 'phil', 'Age' => 16)
554 554
 		));
555 555
 
556 556
 		$list->exclude(array('Name'=>array('bob','phil'),'Age'=>array(10, 16),'Bananas'=>true));
557 557
 		$expected = array(
558  
-			0 => array('Name' => 'bob', 'Age' => 10),
559  
-			1 => array('Name' => 'phil', 'Age' => 11),
560  
-			2 => array('Name' => 'bob', 'Age' => 12),
561  
-			3 => array('Name' => 'phil', 'Age' => 12),
562  
-			4 => array('Name' => 'bob', 'Age' => 14),
563  
-			5 => array('Name' => 'phil', 'Age' => 14),
564  
-			6 => array('Name' => 'bob', 'Age' => 16),
565  
-			7 => array('Name' => 'phil', 'Age' => 16)
  558
+			array('Name' => 'bob', 'Age' => 10),
  559
+			array('Name' => 'phil', 'Age' => 11),
  560
+			array('Name' => 'bob', 'Age' => 12),
  561
+			array('Name' => 'phil', 'Age' => 12),
  562
+			array('Name' => 'bob', 'Age' => 14),
  563
+			array('Name' => 'phil', 'Age' => 14),
  564
+			array('Name' => 'bob', 'Age' => 16),
  565
+			array('Name' => 'phil', 'Age' => 16)
566 566
 		);
567 567
 		$this->assertEquals($expected, $list->toArray());
568 568
 	}
@@ -572,29 +572,29 @@ public function testMultipleExcludeNoMatch() {
572 572
 	 */
573 573
 	public function testMultipleExcludeThreeArguments() {
574 574
 		$list = new ArrayList(array(
575  
-			0 => array('Name' => 'bob', 'Age' => 10, 'HasBananas'=>false),
576  
-			1 => array('Name' => 'phil','Age' => 11, 'HasBananas'=>true),
577  
-			2 => array('Name' => 'bob', 'Age' => 12, 'HasBananas'=>true),
578  
-			3 => array('Name' => 'phil','Age' => 12, 'HasBananas'=>true),
579  
-			4 => array('Name' => 'bob', 'Age' => 14, 'HasBananas'=>false),
580  
-			4 => array('Name' => 'ann', 'Age' => 14, 'HasBananas'=>true),
581  
-			5 => array('Name' => 'phil','Age' => 14, 'HasBananas'=>false),
582  
-			6 => array('Name' => 'bob', 'Age' => 16, 'HasBananas'=>false),
583  
-			7 => array('Name' => 'phil','Age' => 16, 'HasBananas'=>true),
584  
-			8 => array('Name' => 'clair','Age' => 16, 'HasBananas'=>true)
  575
+			array('Name' => 'bob', 'Age' => 10, 'HasBananas'=>false),
  576
+			array('Name' => 'phil','Age' => 11, 'HasBananas'=>true),
  577
+			array('Name' => 'bob', 'Age' => 12, 'HasBananas'=>true),
  578
+			array('Name' => 'phil','Age' => 12, 'HasBananas'=>true),
  579
+			array('Name' => 'bob', 'Age' => 14, 'HasBananas'=>false),
  580
+			array('Name' => 'ann', 'Age' => 14, 'HasBananas'=>true),
  581
+			array('Name' => 'phil','Age' => 14, 'HasBananas'=>false),
  582
+			array('Name' => 'bob', 'Age' => 16, 'HasBananas'=>false),
  583
+			array('Name' => 'phil','Age' => 16, 'HasBananas'=>true),
  584
+			array('Name' => 'clair','Age' => 16, 'HasBananas'=>true)
585 585
 		));
586 586
 
587 587
 		$list->exclude(array('Name'=>array('bob','phil'),'Age'=>array(10, 16),'HasBananas'=>true));
588 588
 		$expected = array(
589  
-			0 => array('Name' => 'bob', 'Age' => 10, 'HasBananas'=>false),
590  
-			1 => array('Name' => 'phil','Age' => 11, 'HasBananas'=>true),
591  
-			2 => array('Name' => 'bob', 'Age' => 12, 'HasBananas'=>true),
592  
-			3 => array('Name' => 'phil','Age' => 12, 'HasBananas'=>true),
593  
-			4 => array('Name' => 'bob', 'Age' => 14, 'HasBananas'=>false),
594  
-			4 => array('Name' => 'ann', 'Age' => 14, 'HasBananas'=>true),
595  
-			5 => array('Name' => 'phil','Age' => 14, 'HasBananas'=>false),
596  
-			6 => array('Name' => 'bob', 'Age' => 16, 'HasBananas'=>false),
597  
-			8 => array('Name' => 'clair','Age' => 16, 'HasBananas'=>true)
  589
+			array('Name' => 'bob', 'Age' => 10, 'HasBananas'=>false),
  590
+			array('Name' => 'phil','Age' => 11, 'HasBananas'=>true),
  591
+			array('Name' => 'bob', 'Age' => 12, 'HasBananas'=>true),
  592
+			array('Name' => 'phil','Age' => 12, 'HasBananas'=>true),
  593
+			array('Name' => 'bob', 'Age' => 14, 'HasBananas'=>false),
  594
+			array('Name' => 'ann', 'Age' => 14, 'HasBananas'=>true),
  595
+			array('Name' => 'phil','Age' => 14, 'HasBananas'=>false),
  596
+			array('Name' => 'bob', 'Age' => 16, 'HasBananas'=>false),
  597
+			array('Name' => 'clair','Age' => 16, 'HasBananas'=>true)
598 598
 		);
599 599
 		$this->assertEquals($expected, $list->toArray());
600 600
 	}

0 notes on commit 9d74c99

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