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.

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

The current collection interface use Closure(TKey=, T=) and Closure(T=, TKey=). #251

Closed
VincentLanglet opened this issue Aug 12, 2020 · 5 comments

Comments

@VincentLanglet
Copy link
Contributor

The closure expected by the method of Collection interface doesn't have the same signature.

For exists, forAll, partition,

@psalm-param Closure(TKey=, T=):bool $p

For filter,

@psalm-param Closure(T=, TKey=):bool $p

For the next major, maybe it could be a nice idea to change the signature of the closure expected by the filter method, in order to have consistency ?

@greg0ire
Copy link
Member

greg0ire commented Aug 24, 2022

@VincentLanglet I'm not sure why you closed this, but IMO filter is like this in order to be consistent with array_filter:

var_dump(array_filter($arr, function($v, $k) {
    return $k == 'b' || $v == 4;
}, ARRAY_FILTER_USE_BOTH));

@VincentLanglet
Copy link
Contributor Author

I'm not even sure the 2.0 branch will be release one day but for retro-compatibility reason I assume an issue like this one will never done.

Adding new methods to 2.0 like #252, still allow library to support 1.x||2.x

But changing this or false return values to null would block supporting 1.x||2.x

@greg0ire
Copy link
Member

I'm not even sure the 2.0 branch will be release one day but for retro-compatibility reason I assume an issue like this one will never done.

I'm looking into this PR because I'm looking into releasing 2.0.0

@VincentLanglet
Copy link
Contributor Author

I'm looking into this PR because I'm looking into releasing 2.0.0

Nice to hear.

Is it any rule about what could be accepted for 2.0 and what won't ?

I see the following issue related to BC-break
For instance #87, #190, #195, ...
And i'll be happy to work on some if they will be accepted.
At least #231 should be considered and easy to do.

@greg0ire
Copy link
Member

Great! We would like to have feature parity between 1 and 2, and ideally have 2 only contain BC breaks, as much as possible, so that it's easy to migrate/support both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants