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.

Sign up for GitHub

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

[8.x] Allow shift() and pop() to take multiple items from a collection #38093

Merged
merged 5 commits into from
Jul 21, 2021
Merged

[8.x] Allow shift() and pop() to take multiple items from a collection #38093

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8575638
Allow shift() and pop() to take multiple items from a collection
edgrosvenor Jul 21, 2021
33f81e7
Make styleci happy
edgrosvenor Jul 21, 2021
81ae758
Add param with typehint to docblock
edgrosvenor Jul 21, 2021
220dd07
formatting
taylorotwell Jul 21, 2021
f2c16e5
formatting
taylorotwell Jul 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 28 additions & 6 deletions src/Illuminate/Collections/Collection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -786,13 +786,24 @@ public function only($keys)
}

/**
* Get and remove the last item from the collection.
* Get and remove the last N items from the collection.
*
* @param int $count
* @return mixed
*/
public function pop()
public function pop($count = 1)
{
return array_pop($this->items);
if ($count === 1) {
return array_pop($this->items);
}

$results = [];

foreach (range(1, $count) as $item) {
array_push($results, array_pop($this->items));
}

return new static($results);
}

/**
Expand Down Expand Up @@ -939,13 +950,24 @@ public function search($value, $strict = false)
}

/**
* Get and remove the first item from the collection.
* Get and remove the first N items from the collection.
*
* @param int $count
* @return mixed
*/
public function shift()
public function shift($count = 1)
{
return array_shift($this->items);
if ($count === 1) {
return array_shift($this->items);
}

$results = [];

foreach (range(1, $count) as $item) {
array_push($results, array_shift($this->items));
Copy link
Member

Choose a reason for hiding this comment

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

array_shift is an expensive operation (as it needs to re-key all items in the array). You don't really want to do that in a loop 😢

}

return new static($results);
}

/**
Expand Down
16 changes: 16 additions & 0 deletions tests/Support/SupportCollectionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,14 @@ public function testPopReturnsAndRemovesLastItemInCollection()
$this->assertSame('foo', $c->first());
}

public function testPopReturnsAndRemovesLastXItemsInCollection()
{
$c = new Collection(['foo', 'bar', 'baz']);

$this->assertEquals(new Collection(['baz', 'bar']), $c->pop(2));
$this->assertSame('foo', $c->first());
}

public function testShiftReturnsAndRemovesFirstItemInCollection()
{
$data = new Collection(['Taylor', 'Otwell']);
Expand All @@ -235,6 +243,14 @@ public function testShiftReturnsAndRemovesFirstItemInCollection()
$this->assertNull($data->first());
}

public function testShiftReturnsAndRemovesFirstXItemsInCollection()
{
$data = new Collection(['foo', 'bar', 'baz']);

$this->assertEquals(new Collection(['foo', 'bar']), $data->shift(2));
$this->assertSame('baz', $data->first());
}

/**
* @dataProvider collectionClassProvider
*/
Expand Down