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

Implement mapByKey #54129

Closed
wants to merge 2 commits into from
Closed

Implement mapByKey #54129

wants to merge 2 commits into from

Conversation

driesvints
Copy link
Member

@driesvints driesvints commented Jan 9, 2025

This PR adds a new array helper and collection method called mapByKey. With this method instead of this:

$result = Arr::map($array, function ($value, $key) {
    return in_array($key, ['foo', 'bar']) ? strtolower($value) : $value;
});

You can do:

$result = Arr::mapByKey($array, ['foo', 'bar'], fn ($value) => strtolower($value));

So basically target any key in a given item on an array and manipulate it.

Things not added yet which I'd love to get some help with:

  • Verify if LazyCollection support is implemented correctly
  • Generics in docblocks
  • Deep nested key targeting with dot notation
  • Enumerable contract method (can be added on master once it's merged upstream)

*/
public function mapByKey($keys, callable $callback)
{
return $this->passthru('mapByKey', func_get_args());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% sure this is the correct way to go about adding a method to LazyCollection.

@driesvints
Copy link
Member Author

@josepostiga would be cool if you could give some feedback on what can be improved here.

@shaedrich
Copy link
Contributor

This PR adds a new array helper and collection method called mapByKey

I'm not sure if that is the ideal naming. To me, it does not exactly convey what it does. Actually, it's doing something more along the lines of mapSpecificKeys

@josepostiga
Copy link

@josepostiga would be cool if you could give some feedback on what can be improved here.

Sorry, didn't mean to cause entropy in your development/thinking process 😅

I think the behavior is interesting. I just think the name could be a little different? Maybe mapFilteringByKey?

@RVxLab
Copy link
Contributor

RVxLab commented Jan 9, 2025

I think this could be a very nice feature. I’m just a bit confused by the name. To me, on its face, looks like another way to map but the key is the first argument.

What about mapOnly for the name? It would fit the names on Eloquent models, collections and the array helper. Inversely that could lead to mapExcept.

@taylorotwell
Copy link
Member

Going to skip this one for now.

@taylorotwell taylorotwell deleted the map-by-keys branch January 9, 2025 19:45
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.

5 participants