refactor: Refactor `Reduce` operation.

loophp / collection

A (memory) friendly, easy, lazy and modular collection class.

https://loophp-collection.rtfd.io/

MIT License

721 stars 35 forks source link

refactor: Refactor `Reduce` operation. #259

Closed drupol closed 2 years ago

drupol commented 2 years ago

BREAKING CHANGE: yes

This PR:

[x] Fix #256
[x] It breaks backward compatibility
[x] Is covered by unit tests
[x] Has static analysis tests (psalm, phpstan)
[x] Has documentation
[ ] Is an experimental thing

DONE 🟢

[x] first
[x] foldLeft
[x] foldLeft1
[x] foldRight
[x] foldRight1
[x] implode
[x] reduce
[x] unlines
[x] unwords
[x] last
[x] head
[x] get
[x] compare
[x] min
[x] max

drupol commented 2 years ago

I think I'm done with this PR, let's leave it open a little bit to carefully check if this is the proper thing to do.

I know the pace is a bit different during holidays, so, it might take a little while before it gets merged.

drupol commented 2 years ago

@AlexandruGG I think it's ready, wdyt ?

AlexandruGG commented 2 years ago

@AlexandruGG I think it's ready, wdyt ?

Amazing! I'll go through this today :)

AlexandruGG commented 2 years ago

@AlexandruGG I think it's ready, wdyt ?

@drupol let me know if you'd like me to help with the remaining ones :)

drupol commented 2 years ago

Thanks for the review Alex, I can continue to work on it tonight, but feel free to push things if you want to.

AlexandruGG commented 2 years ago

Thanks for the review Alex, I can continue to work on it tonight, but feel free to push things if you want to.

Sure, leave the comparison operations to me :)

drupol commented 2 years ago

I now leave this PR to you @AlexandruGG , good work !

AlexandruGG commented 2 years ago

I now leave this PR to you @AlexandruGG , good work !

Thanks! I added the remaining operations and updated a couple bits of documentation that were still mentioning current.

I'll give this another fresh look tomorrow, see if I can spot anything else needed.

drupol commented 2 years ago

Thanks! I added the remaining operations and updated a couple bits of documentation that were still mentioning current.

Indeed, nice. I was planning to do it after all the commits were done, but you did it. Thanks :)

I'll give this another fresh look tomorrow, see if I can spot anything else needed.

Ok will do that ! Cheers!

AlexandruGG commented 2 years ago

@drupol I went through this again and added a few more things. Please feel free to comb through the docs yourself as well and see if you can spot any bits we might have missed

drupol commented 2 years ago

This is super great !!!

I propose to merge this, then do remaining changes if any in another branch.

We've been waiting for too long for this :)