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

feat: add plus operation #314

Closed
wants to merge 6 commits into from
Closed

feat: add plus operation #314

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fa73667
feat: add `plus` operation
drupol Nov 11, 2023
9f29100
add tests
drupol Nov 12, 2023
1026d49
fix: add missing implementation
drupol Nov 13, 2023
7533c47
update implementation
drupol Nov 13, 2023
c633b7e
fix: phpstan baseline update
drupol Nov 13, 2023
90943dd
fix: variable name
drupol Nov 13, 2023
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
10 changes: 10 additions & 0 deletions phpstan-baseline.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,16 @@ parameters:
count: 1
path: src/Operation/Map.php

-
message: "#^Template type U of method loophp\\\\collection\\\\Operation\\\\Plus\\:\\:__invoke\\(\\) is not referenced in a parameter\\.$#"
count: 1
path: src/Operation/Plus.php

-
message: "#^Template type UKey of method loophp\\\\collection\\\\Operation\\\\Plus\\:\\:__invoke\\(\\) is not referenced in a parameter\\.$#"
count: 1
path: src/Operation/Plus.php

-
message: "#^Template type U of method loophp\\\\collection\\\\Operation\\\\Prepend\\:\\:__invoke\\(\\) is not referenced in a parameter\\.$#"
count: 1
Expand Down
5 changes: 5 additions & 0 deletions src/CollectionDecorator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,11 @@ public function pluck(mixed $pluck, mixed $default = null): static
return new static($this->innerCollection->pluck($pluck, $default));
}

public function plus(iterable $items): static
{
return new static($this->innerCollection->plus($items));
}

public function prepend(mixed ...$items): static
{
return new static($this->innerCollection->prepend(...$items));
Expand Down
2 changes: 2 additions & 0 deletions src/Contract/Collection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@
* @template-extends Operation\Permutateable<TKey, T>
* @template-extends Operation\Pipeable<TKey, T>
* @template-extends Operation\Pluckable<TKey, T>
* @template-extends Operation\Plusable<TKey, T>
* @template-extends Operation\Prependable<TKey, T>
* @template-extends Operation\Productable<TKey, T>
* @template-extends Operation\Randomable<TKey, T>
Expand Down Expand Up @@ -213,6 +214,7 @@ interface Collection extends
Operation\Permutateable,
Operation\Pipeable,
Operation\Pluckable,
Operation\Plusable,
Operation\Prependable,
Operation\Productable,
Operation\Randomable,
Expand Down
28 changes: 28 additions & 0 deletions src/Contract/Operation/Plusable.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

declare(strict_types=1);

namespace loophp\collection\Contract\Operation;

use loophp\collection\Contract\Collection;

/**
* @template TKey
* @template T
*/
interface Plusable
{
/**
* TODO - Plus items.
*
* @see https://loophp-collection.readthedocs.io/en/stable/pages/api.html#plus
*
* @template UKey
* @template U
*
* @param iterable<UKey, U> $items
*
* @return Collection<int|TKey|UKey, T|U>
*/
public function plus(iterable $items): Collection;
}
53 changes: 53 additions & 0 deletions src/Operation/Plus.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php

declare(strict_types=1);

namespace loophp\collection\Operation;

use Closure;
use Generator;
use loophp\iterators\ConcatIterableAggregate;

/**
* @immutable
*
* @template TKey
* @template T
*/
final class Plus extends AbstractOperation
{
/**
* @template UKey
* @template U
*
* @return Closure(iterable<UKey, U>): Closure(iterable<TKey, T>): iterable<int|TKey|UKey, T|U>
*/
public function __invoke(): Closure
{
$comparatorCallback =
/**
* @param T $left
*
* @return Closure(T): bool
*/
static fn (mixed $left): Closure =>
/**
* @param T $right
*/
static fn (mixed $right): bool => $left === $right;

return
/**
* @param iterable<UKey, U> $items
*
* @return Closure(iterable<TKey, T>): iterable<int|TKey|UKey, T|U>
*/
static fn (iterable $items): Closure =>
/**
* @param iterable<TKey, T> $iterable
*
* @return Generator<int|TKey|UKey, T|U>
*/
static fn (iterable $iterable): Generator => yield from (new Distinct())()($comparatorCallback)(static fn (mixed $value, mixed $key): mixed => $key)(new ConcatIterableAggregate([$iterable, $items]));
Copy link
Contributor

Choose a reason for hiding this comment

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

@drupol , does it make any sense to remove duplicated indexes in the existing collection? I assume it would be more apparent if these were kept. Consider following test (fails with current implementation):

public function testPlusDoesNotRemoveExistingDuplicatedIndices(): void
{
    $collection = Collection::fromIterable([
        ['id' => 1],
        ['id' => 2],
    ]);

    $result = $collection
        ->unwrap()
        ->plus(['id' => 3, 'id2' => 4])
        ->wrap()
        ->all(false)
    ;

    self::assertSame([
        ['id' => 1],
        ['id' => 2],
        ['id2' => 4],
    ], $result);
}

Moreover, it would be really pity if plus([]) would drop all key duplicates. The following test fails as well:

public function testPlusEmptyArrayDoesNotModifyCollection(): void
{
    $collection = Collection::fromIterable([
        ['id' => 1],
        ['id' => 2],
    ]);

    $result = $collection
        ->unwrap()
        ->plus([])
        ->wrap()
        ->all(false)
    ;

    self::assertSame([
        ['id' => 1],
        ['id' => 2],
    ], $result);
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Very nice catch and test case. I will see how we can improve the implementation. Feel free to come up with ideas.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I updated the tests and the implementation, this is a bit uglier, but AFAIK there's no other alternative.

}
}
1 change: 1 addition & 0 deletions tests/unit/CollectionGenericOperationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ final class CollectionGenericOperationTest extends TestCase
* @dataProvider permutateOperationProvider
* @dataProvider pipeOperationProvider
* @dataProvider pluckOperationProvider
* @dataProvider plusOperationProvider
* @dataProvider prependOperationProvider
* @dataProvider productOperationProvider
* @dataProvider reductionOperationProvider
Expand Down
1 change: 1 addition & 0 deletions tests/unit/CustomCollectionGenericOperationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ final class CustomCollectionGenericOperationTest extends TestCase
* @dataProvider permutateOperationProvider
* @dataProvider pipeOperationProvider
* @dataProvider pluckOperationProvider
* @dataProvider plusOperationProvider
* @dataProvider prependOperationProvider
* @dataProvider productOperationProvider
* @dataProvider reductionOperationProvider
Expand Down
50 changes: 50 additions & 0 deletions tests/unit/Traits/GenericCollectionProviders.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3291,6 +3291,56 @@ public static function pluckOperationProvider()
];
}

public static function plusOperationProvider()
{
$output = static function (): Generator {
yield 0 => 'A';

yield 1 => 'B';

yield 2 => 'C';
};

yield [
'plus',
[range('A', 'C')],
[],
$output(),
];

$output = static function (): Generator {
yield 0 => 'A';

yield 1 => 'B';

yield 2 => 'C';
};

yield [
'plus',
[[]],
range('A', 'C'),
$output(),
];

$output = static function (): Generator {
yield 0 => 'A';

yield 1 => 'B';

yield 2 => 'E';

yield 3 => 'F';
};

yield [
'plus',
[range('C', 'F')],
range('A', 'B'),
$output(),
];
}

public static function prependOperationProvider()
{
$output = static function (): Generator {
Expand Down
Loading