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

Add Collection::timesWithKeys method #19276

Closed
wants to merge 2 commits into from
Closed

Add Collection::timesWithKeys method #19276

wants to merge 2 commits into from

Conversation

CupOfTea696
Copy link
Contributor

@CupOfTea696 CupOfTea696 commented May 19, 2017

Create a new collection with keys, by invoking a callback a given amount of times.

Example usage:

Collection::times(2, function ($number) {
    return ['slug-'.$number => 'Title '.$number];
})->toArray();

/**
 *   [
 *       ['slug-1' => 'Title 1'],
 *       ['slug-2' => 'Title 2']
 *   ]
 */

Create a new collection with keys, by invoking a callback a given amount of times
@JosephSilber
Copy link
Member

JosephSilber commented May 19, 2017

First off, you have a typo in your example. You want to use timesWithKeys, not times.


The method name timesWithKeys doesn't make much sense to me.

If you need this, maybe we shouldn't require a callback to the times method:

public static function times($amount, callable $callback = null)
{
    if ($amount < 1) {
        return new static;
    }

    $numbers = new static(range(1, $amount));

    return $callback ? $numbers->map($calback) : $numbers;
}

Then you can call mapWithKeys yourself:

Collection::times(2)->mapWithKeys(function ($number) {
    return ['slug-'.$number => 'Title '.$number];
});

@taylorotwell
Copy link
Member

I don't really like the naming of this. I think Joseph's suggestion is maybe a bit better.

@CupOfTea696 CupOfTea696 deleted the feature/collection-times-with-keys branch May 19, 2017 13:41
@CupOfTea696
Copy link
Contributor Author

I'll make a new PR, this does sound like a better idea, thanks!

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.

3 participants