As we are converting all the things to utility classes/OO it makes sense to also do the same from drupal_map_assoc. Give it a nice new home with the other utility classes.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

damiankloip’s picture

Assigned: Unassigned » damiankloip

Working on this.

damiankloip’s picture

Status: Active » Needs review
FileSize
58.87 KB

I have created a new MapArray utility class. We also have DiffArray and NestedArray. Could this live in with one of those? or is an explicit class better? or could we consolidate with DiffArray and have Array{something} (whatever name) and include diffAssocRecursive and mapAssoc methods?

Status: Needs review » Needs work

The last submitted patch, 2004798.patch, failed testing.

damiankloip’s picture

Status: Needs work » Needs review
FileSize
58.7 KB
dawehner’s picture

+++ b/core/lib/Drupal/Component/Utility/MapArray.phpundefined
@@ -0,0 +1,46 @@
+class MapArray {

We have a string utility so what about just use Array here?

+++ b/core/lib/Drupal/Component/Utility/MapArray.phpundefined
@@ -0,0 +1,46 @@
+  public static function mapAssoc(array $array, $callable = NULL) {

So what about Array::map ?

Could you find a test for this function? This would be a perfect candidate for a unit test.

Status: Needs review » Needs work

The last submitted patch, 2004798-4.patch, failed testing.

damiankloip’s picture

Status: Needs work » Needs review
FileSize
62 KB

We have a string utility so what about just use Array here?

Alas, we can't name a class 'Array' in php... So how about DrupalArray? Also changed the method name just to 'map'.

Yeah, I don't think this is teste at all yet, so added some unit tests.

Status: Needs review » Needs work

The last submitted patch, 2004798-6.patch, failed testing.

damiankloip’s picture

Status: Needs work » Needs review
FileSize
1.04 KB
62.23 KB

Somehow that one slipped through the net, even though I changed the name with PHPStorm.... ;)

Status: Needs review » Needs work

The last submitted patch, 2004798-9.patch, failed testing.

dawehner’s picture

+++ b/core/tests/Drupal/Tests/Component/Utility/DrupalArrayTest.phpundefined
@@ -0,0 +1,69 @@
+    $test_arrays = array(
+      array('one', 'two', 'three'),
+      array(3, 4, 5),
+    );
+
+    // Test strings and integers get mapped.
+    foreach ($test_arrays as $test_array) {
+      $processed_array = DrupalArray::map($test_array);
+
+      $this->assertCount(count($test_array), $processed_array);
+      foreach ($test_array as $value) {
+        $this->assertArrayHasKey($value, $processed_array);
+        $this->assertContains($value, $processed_array);
+      }
+
+      // Test passing a callable works as expected.
+      $processed_array = DrupalArray::map($test_array, function ($item) {
+        return 'test';
+      });
+
+      $this->assertCount(count($test_array), $processed_array);
+      foreach ($test_array as $value) {
+        $this->assertArrayHasKey($value, $processed_array);
+        $this->assertEquals($processed_array[$value], 'test');
+      }
+    }
+
+    // Passing an invalid callable should return the originally mapped array.
+    $processed_array = DrupalArray::map($test_array, 'invalid_callable');
+
+    $this->assertCount(count($test_array), $processed_array);
+    foreach ($test_array as $value) {
+      $this->assertArrayHasKey($value, $processed_array);
+      $this->assertContains($value, $processed_array);

It seems to be that this is a good usecase for dataproviders.

ParisLiakos’s picture

Status: Needs work » Closed (duplicate)