Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[9.x] Add keyBy() method to Arr helpers #41029

Merged
merged 6 commits into from
Feb 15, 2022

Conversation

medeirosinacio
Copy link
Contributor

@medeirosinacio medeirosinacio commented Feb 15, 2022

Hi guys, this is my first PR. Please let me know if you have anything to change or improve. This method was inspired by the ArrayHelper::index of the Yii2 framework. I used this method a lot to format arrays and I would really like laravel to have it too

Indexes the array according to a specified key.

use case

$array = [
    ['id' => '123', 'data' => 'abc', 'device' => 'laptop'],
    ['id' => '345', 'data' => 'def', 'device' => 'tablet'],
    ['id' => '345', 'data' => 'hgi', 'device' => 'smartphone'],
];
$result = Arr::index($array, 'id');

The result will be an associative array, where the key is the value of id attribute

[
    '123' => ['id' => '123', 'data' => 'abc', 'device' => 'laptop'],
    '345' => ['id' => '345', 'data' => 'hgi', 'device' => 'smartphone']
    // The second element of an original array is overwritten by the last element because of the same id
]
  • Renamed Arr:index to Arr:keyBy

@GrahamCampbell GrahamCampbell changed the title Add index() method to Arr helpers [9.x] Add index() method to Arr helpers Feb 15, 2022
@pyrou
Copy link
Contributor

pyrou commented Feb 15, 2022

About naming, same behavior exists on Collection, its named keyBy

@medeirosinacio
Copy link
Contributor Author

About naming, same behavior exists on Collection, its named keyBy

Yes true! I can change the name of the method to leave both with the same behavior. It would be interesting for the Arr class to also have this method.

@dennisprudlo
Copy link
Contributor

dennisprudlo commented Feb 15, 2022

If this array helper and the collection function are supposed to do the same we maybe should simply use

collect($array)->keyBy('id')

or if merged update the keyBy function of the collection to use the Arr::index() helper. Otherwise we'd end up with two different versions of this kind of function.

@taylorotwell
Copy link
Member

If we already have a collection method that does the exact same thing then this method should be named the same as well.

@medeirosinacio medeirosinacio changed the title [9.x] Add index() method to Arr helpers [9.x] Add keyBy() method to Arr helpers Feb 15, 2022
@medeirosinacio
Copy link
Contributor Author

If we already have a collection method that does the exact same thing then this method should be named the same as well.

Hello, I refactored the method to use the function that already exists in the collection.

Let Me know if anything is missing.

@medeirosinacio
Copy link
Contributor Author

If this array helper and the collection function are supposed to do the same we maybe should simply use

collect($array)->keyBy('id')

or if merged update the keyBy function of the collection to use the Arr::index() helper. Otherwise we'd end up with two different versions of this kind of function.

Yes, I made the change so that the method uses the existing function in the collection

@taylorotwell taylorotwell merged commit 4b478a6 into laravel:9.x Feb 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants