Plan for v5 - BC Break release

[drupol]

Release date proposal: 6th of August, exactly 2 years after the first commit (7247de4)!

Ideas for v5:

• Variadic static constructors ? #129 (could be done later)
• Get rid of IterableIterator ? (would be nice!)

Ongoing:

• Writing a banging changelog!

Implemented/Done:

• Make constructor private ? refactor: Make constructor private. #163
• Deprecate ::mapN() ?
• What should we do with ::first(), ::head() ? They are doing the same thing. Should we deprecate ::first() ?
• Increase typing here and there, reach 97% ?
• Add Reduce operation #139
• Get rid of Todo? (Update Reverse operation - optimize and remove todos. #138)

Not going to do it:

• Replace Iterator with iterable in Operations. ?
• Introduce helpers for static constructors #158

[AlexandruGG]

• mapN -> why deprecate? I think it's useful, are you thinking to remove it eventually?
• first, head -> we could say the same about last and tail, however, I don't mind the duplication. I think for people coming from FP, head and tail will be familiar. However, for others first and last are more self-explanatory. And other popular collections from Doctrine and Laravel use them

[AlexandruGG]

speaking of map, should we apply the same treatment to asyncMap? I.e. have asyncMapN and make the other one accept a single callback?

[drupol]

I think this is a good idea, yeah.

[AlexandruGG]

Another thought here: I think we should aim to solve the current TODOs in the project if possible. Or just remove them from the code if we deem them as not a priority anymore, and list them somewhere else in a discussion/issue

[drupol]

One less ! It was bugging me since quite a long time, the solution was easy :) #138

[AlexandruGG]

@drupol Two more thoughts for this, as I've been refactoring more to get rid of Doctrine ArrayCollection and use loophp Collection:

1. What do you think of having helpers à la Laravel? More specifically, I'm thinking of instantiation methods in order to avoid having too many imports. Here's an example, imagine I have a class which has methods that use both collection types:

use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\Common\Collections\Collection as DoctrineCollection;
use loophp\collection\Collection as CollectionInstance;
use loophp\collection\Contract\Collection;

class Service {
    public function returnsDoctrine(): DoctrineCollection 
    {
        return new ArrayCollection([1, 2, 3]);
    }

    public function returnsLoophp(): Collection // I want to keep the Contract here i.e. the Interface
    {
        return CollectionInstance::fromIterable([1, 2, 3]);
    }

    public function returnsLoophp2(): Collection 
    {
        return collectIterable([1, 2, 3]); 
        // having this would allow me to get rid of the instance import;
        // also, it's easier and shorter to write :)
        // we could have equivalents `collectString`, `collectResource` etc.
    }
}

2. It would be very helpful to have an equals method, have you ever considered it?

// signature
// Collection::equals(Collection $other): bool

// usage
$col1 = Collection::fromIterable([1, 2, 3]);
$col2 = Collection::fromIterable([3, 4, 5]);

if ($col1->equals($col2)) {
}

I'm thinking it would be implemented as Pipe()(Diff(), IsEmpty()).

The other major benefit is it would allow usage in testing with PHUnit's assertObjectEquals assertion. I have many methods that return collections and currently it's a bit cumbersome to test the return values because using all is not always correct, other operations like normalize need to be applied sometimes in order to not lose all the items.

[drupol]

Why not! Let's make a PR per idea and discuss about them !

[AlexandruGG]

@drupol another one potentially for this release

currently many interfaces have a short explanation in the dock block:
https://github.com/loophp/collection/blob/master/src/Contract/Operation/Allable.php
https://github.com/loophp/collection/blob/master/src/Contract/Operation/Mapable.php

but many do not:
https://github.com/loophp/collection/blob/master/src/Contract/Operation/Diffable.php
https://github.com/loophp/collection/blob/master/src/Contract/Operation/Keyable.php

Should we:
A. remove the description from the existing ones
B. add a description from the one that do not have one

If we are going with B, again we have a few options:

1. write a short description like the other ones
2. copy the description from the api docs
3. just add a link to the api docs instead of copying the description:

interface Diffable
{
    /**
     * @link https://loophp-collection.readthedocs.io/en/stable/pages/api.html#diff
     *
     * @param T ...$values
     *
     * @return Collection<TKey, T>
     */
    public function diff(...$values): Collection;
}

4. copy the description from the api docs and add the link:

interface Diffable
{
    /**
     * It compares the collection against another collection or a plain array based on its values. 
     * This method will return the values in the original collection that are not present in the given collection.
     * @link https://loophp-collection.readthedocs.io/en/stable/pages/api.html#diff
     *
     * @param T ...$values
     *
     * @return Collection<TKey, T>
     */
    public function diff(...$values): Collection;
}

[drupol]

Yeah basically I don't know what to choose yet.

[AlexandruGG]

sounds like we should go for B.4 then?

[drupol]

I have the feeling that this is the best for now, no ?

[AlexandruGG]

yep sounds good, more documentation that's easily accessible is always great. I'll do a PR for that

[drupol]

Ok, I let you do that!

[drupol]

The other day I had an idea.

Instead of using Operation::of(), maybe we should use (new Operation)().

Benefits:

• We do not loose typing information because ::of() is declared in abstract operation class.

Cons:

• More characters to type, more parenthesis, more confusion.

WDYT?

[AlexandruGG]

Hmm would be interesting to try on one of the operations and see if the typing information works as you suspect. If it does it could be a great benefit.

More parenthesis is annoying indeed but I think the benefit outweighs it

[drupol]

Something like this: https://psalm.dev/r/1bca9e9765 but I don't know yet how to fix that... and PHPStan shows no errors with this! https://phpstan.org/r/e434bd69-ae22-449e-9035-0257fb167b51

[AlexandruGG]

PHPstan doesn't have as advanced analysis around purity and immutability I think.

To solve those we need to mark the relevant Closures as pure as well: https://psalm.dev/r/785520d3b9. This doesn't solve everything but you can see it's on the right track.

This refactoring would be a massive work to get it right, maybe a future version 😄 .

I was thinking it would be really good to have separate static analysis tests and phpspec tests for the Operations themselves, not just the Collection class. However, I am not that interested in using them separately to be fair 😅 .

[AlexandruGG]

@drupol I just noticed this one:

Make constructor private ?

I think it would be good to do for sure!

[drupol]

Yeah I sometimes update the top messages and I guess you don't receive notifications!

Anyway, when I do that I'll post a message so you receive a notification.

I just updated it :)

[drupol]

I think it's time to focus on writing the Changelog :D