Closed Smoren closed 1 year ago
This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Totals | |
---|---|
Change from base Build 4171111633: | 0.002% |
Covered Lines: | 687 |
Relevant Lines: | 688 |
Hi @Smoren,
Thank you for the PR to improve the groupBy
functionality.
If I understand it correctly, there are two changes here:
Does that sound correct or would you describe it differently? Thanks, Mark
Hi @markrogoyski,
Yes, completely true!
I've refactored Single::groupBy()
a little bit: using of Stream::of($group)->toArray()
replaced by using Transform::toArray($group)
.
This branch is rebased from develop
.
Hi @markrogoyski,
What's about this PR? I really need this functionality at my job. I will be glad if it will be included in the next release.
Hi @Smoren,
Sorry for the delay in reviewing this.
I think there may be a bug if there are duplicated candidate group items. For example, try this unit test case for testArray
:
[
[
['name' => 'Sam', 'interests' => ['programming', 'books', 'slacking', 'music']],
['name' => 'Laura', 'interests' => ['math', 'fantasy', 'wine', 'music']],
['name' => 'Alice', 'interests' => ['music', 'music', 'programming', 'fantasy']], // Alice has "music" twice
['name' => 'Anonymous', 'interests' => []],
],
fn ($x) => $x['interests'],
null,
[ // ???
],
],
I don't think it will deduplicate the results, and Alice will end up in the music group twice.
If you add $itemKeyFunction
then I think it will overwrite that key and only show up once, but the interest will still show up twice.
You can argue it is perhaps bad data, but the resultant group should not end up with more participants than were originally input. I think you might want to try to dedupe this somehow.
Hi @markrogoyski,
I've implemented prevention of duplicating items in groups and added new test cases.
Thank you for merging this PR!
Hi @Smoren,
I've released this and the other new functionality in the latest release v1.4.0. Thank you for all the PRs and implementing new features!
Mark
Hi @markrogoyski, this is the great news! Thank you!
Hi @markrogoyski, I think there is a formatting typo here in the release description.
Thanks. I fixed it.
Hi @markrogoyski,
Solving a specific task, I came across the fact that I lack the functionality of the method
Single::groupBy()
.So I've improved
Single::groupBy()
andStream::groupBy()
methods with$groupKeyFunction
param and multiple item groups support.Usage example: