loophp/collection

Let users customize how to access a value in the Distinct operation.

drupol opened this issue · 5 comments

The user should be able to use the distinct operation with a collection of object.

Currently, we use === in the Distinct operation to compare values, however, it happens sometimes that we want to customize
how to access to the values to compare.

Code example

<?php

/**
 * For the full copyright and license information, please view
 * the LICENSE file that was distributed with this source code.
 */

declare(strict_types=1);

namespace App;

use loophp\collection\Collection;

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

class User {
    private string $name;

    public function __construct(string $name)
    {
        $this->name = $name;
    }

    public function name(): string
    {
        return $this->name;
    }
};

$users = [
    new User('a'),
    new User('b'),
    new User('a'),
    new User('c'),
];

$collection = Collection::fromIterable($users)
    ->distinct();

print_r($collection->all());

In this particular case, all the users will be returned and the Distinct function will not work.

How to fix?

The idea is to let users customize how values should be accessed and/or compared.

Proposal 1

Let users customize how to access to the value to compare.

$collection = Collection::fromIterable($users)
    ->distinct(static fn(User $user): string => $user->name());

Proposal 2

Let users customize how to access to the value to compare and how to compare them.

$collection = Collection::fromIterable($users)
    ->distinct(static fn(User $left, User $right): bool => $left->name() === $right->name());

Proposal 3

Mix the two previous proposals:

$collection = Collection::fromIterable($users)
    ->distinct(
        static fn(User $user): string => $user->name(),
        static fn(string $left, string $right): bool => $left === $right
    ),

I tested both proposals and I'm unable to decide which way to go.

Therefore, this issue is to decide on which way to go.

This is a good idea, Proposal 2 sounds the best to me because there might be more than a single element with objects which will determine equality.

For example, with the Money object from moneyphp, you need to compare both the amount and the currency. It already has an equals method: https://github.com/moneyphp/money/blob/master/src/Money.php#L112-L126.

In addition, PHPUnit also promotes object comparisons via user-defined equals methods, which will be used with this assertion: https://phpunit.readthedocs.io/en/9.5/assertions.html#assertobjectequals. So users might already have this method on many of their value objects.

Proposal 2 allows for both these usages:

Collection::fromIterable($users)
    ->distinct(static fn(User $left, User $right): bool => $left->equals($right));

Collection::fromIterable($users)
    ->distinct(static fn(User $left, User $right): bool => $left->name() === $right->name && $left->email() === $right->email());

Okay, that was my preference as well.

I added a third proposal, which might be an option as well, do you think it would be better?

I implemented the proposal 3 locally so far, I like it. It's fully customizable and the logic is properly decoupled.

WDYT?

I like the third approach, best of both!

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.