loophp / collection

A (memory) friendly, easy, lazy and modular collection class.
https://loophp-collection.rtfd.io/
MIT License
721 stars 35 forks source link

Plus operation RFC #313

Closed rela589n closed 10 months ago

rela589n commented 11 months ago

PHP plus operator

I'm wondering if there's same feature as php plus operator:

public function testPlusOperation(): void
{
    $initialArray =[
        'id' => ['A', 'B'],
        'id2' => ['C'],
    ];

    $resultArray = $initialArray + ['id' => [], 'id2' => [], 'id3' => []];

    self::assertSame([
        'id' => ['A', 'B'],
        'id2' => ['C'],
        'id3' => [],
    ], $resultArray);
}

As of me, personally I often use plus operation on arrays and it would make sense to implement it within a collection.

How it'd look like with collection

public function testCollectionPlusOperation(): void
{
    $initialCollection = Collection::fromIterable([
        ['id' => 'A'],
        ['id' => 'B'],
        ['id2' => 'C'],
    ]);

    $result = $initialCollection
        ->unwrap()
        ->groupBy(static fn ($value, $key) => $key)
        ->plus(['id' => [], 'id2' => [], 'id3' => []])
        ->all(false)
    ;

    self::assertSame([
        'id' => ['A', 'B'],
        'id2' => ['C'],
        'id3' => [],
    ], $result);
}

Comparison with prepend

With current implementation seemingly the same behavior may be achieved using prepend:

$result = $initialCollection
    ->unwrap()
    ->groupBy(static fn ($value, $key) => $key)
    ->prepend(...['id' => [], 'id2' => [], 'id3' => []])
    ->all(false)
;

self::assertSame([
    'id' => ['A', 'B'],
    'id2' => ['C'],
    'id3' => [],
], $result);

However it is not explicit solution, since the collection maintains all the keys until all() is called.

public function testPrependedKeysAreStillPresent(): void
{
    $initialCollection = Collection::fromIterable([
        ['id' => 'A'],
        ['id' => 'B'],
        ['id2' => 'C'],
    ]);

    $result = $initialCollection
        ->unwrap()
        ->groupBy(static fn ($value, $key) => $key)
        ->prepend(...['id' => [], 'id2' => [], 'id3' => []])
        ->pack()
        ->all()
    ;

    self::assertSame([
        ['id', []],
        ['id2', []],
        ['id3', []],
        ['id', ['A', 'B']],
        ['id2', ['C']],
    ], $result);
}

Therefore, when using prepend, we must convert collection into array before we can safely use the result, lest same instance iteration would yield id and id2 keys twice and id3 key before the rest.

Please, let me know what you think about this plus() proposal.

drupol commented 11 months ago

Hi there!

Super cool idea !

Are you willing to submit a PR?

Thanks

rela589n commented 11 months ago

Hi Pol, thanks for quick response!

Great to hear that!

Well, currently I have zero experience with functional programming and I doubt if this task is feasible for me. At least for now, this library looks like a black box with some magic inside. Not because something is wrong with it, - conversely it is me who is FP freshmen.

Thus, there will be no pull request from my side soon.

drupol commented 11 months ago

OK I understand, no problem! I'll look into it when I have a bit of time.

Sponsoring is greatly appreciated though.

drupol commented 11 months ago

Dear @rela589n,

I've implemented the feature in #314.

Would you be OK to test it?

Thanks!

rela589n commented 11 months ago

It would be nice

Though, I'm not sure where to start from. I assume at first I should check documentation page https://loophp-collection.readthedocs.io/en/latest/pages/tests.html , but what should I do then? Maybe you could point out some similar test(s) examples I can inspire implementation from. And "Static analysis tests" - something really new to me.

Thank you

drupol commented 11 months ago

You could simply checkout the branch containing the new operation and create a simple file in the root of the project test.php, containing:

<?php

declare(strict_types=1);

namespace Test;

use loophp\collection\Collection;

require_once __DIR__ . '/vendor/autoload.php';

$collection = Collection::fromIterable([1, 2, 3])
    ->plus([4, 5, 6])
    ->plus(['a' => 'a']);

foreach ($collection as $i => $r) {
    var_dump($r);
}

So that you can play with the new operation and see if it corresponds to what you were expecting.

rela589n commented 11 months ago

I've checked it.

Works like a charm! It does exactly what is expected.

If you wish to add some unit tests, as a starting point you may use the following test case from my branch

https://github.com/loophp/collection/compare/feat/add-plus-operation...rela589n:loophp-collection:feat/add-plus-operation

Thank you!

drupol commented 11 months ago

Thanks! I'm using a specific workflow for tests based on data providers. I made a new commit for tests, find it here: https://github.com/loophp/collection/pull/314/commits/9f291006bc56c2d8122d33c4a1bf462718a16c8b

What would really help me is to create the documentation and the code example.

github-actions[bot] commented 11 months ago

Since this issue has not had any activity within the last 5 days, I have marked it as stale. I will close it if no further activity occurs within the next 5 days.

drupol commented 10 months ago

@rela589n Are you willing to contribute to the documentation of the associated PR ?