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

Add RecursiveIterableAggregate #61

Merged
merged 3 commits into from
Sep 26, 2024
Merged

Add RecursiveIterableAggregate #61

merged 3 commits into from
Sep 26, 2024

Conversation

bobkorinek
Copy link
Contributor

Hello,

First, I'd like to say: great library you have here!

I was hoping to contribute by adding RecursiveIterableAggregate for traversing tree-like structures, but I didn't check the open pull requests first and, ironically, found a very similar PR already in progress. Since the work I've done isn't much, feel free to discard this PR if you find it unnecessary.

I’m not sure if the class name is correct, but I’ve tried to find inspiration in MapIterableAggregate and model this class similarly. I’ve also aimed to pass the callback arguments in the same way that MapIterableAggregate does (i.e., passing value, key, and the original iterable).

@bobkorinek bobkorinek requested a review from drupol as a code owner September 25, 2024 12:38
Copy link

what-the-diff bot commented Sep 25, 2024

PR Summary

  • Introduction of New Class
    We've introduced a new class called RecursiveIterableAggregate. This class complies with IteratorAggregate, a standard set of rules for creating objects that can be looped through - or "iterated" - in a particular fashion.

  • Nesting and Flattening Capabilities
    The new class constructor is now able to take an iterable (a data structure that can be looped through) and a specific criteria - represented as a closure - that's utilized to streamline or 'flatten' nested data structures. This simplifies complex data for more efficient processing.

  • Iterate with Efficiency
    We've developed the getIterator method. This allows for iterative access - allowing us to go through data piece by piece - to yield or output the flattened data from the supplied iterable.

  • Added Recursive Flattening
    A private method flatten has been added. This method is designed to recursively flatten iterable data based on the provided closure logic.

  • Testing the New Class
    A new test class named RecursiveIterableAggregateTest has been created to ensure the RecursiveIterableAggregate class functions as expected.

  • Test Data Provider
    We've included a method provideBasicCases in our test class. This method offers sets of input and corresponding expected output for testing elementary cases.

  • Test Execution
    The testBasic method has been implemented to test basic use cases. It asserts or verifies that the expected results match the output from the RecursiveIterableAggregate iterable process.

Copy link
Contributor

@drupol drupol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this PR, very nice!

I added a comment for performance, let me know if this could be improved.

src/RecursiveIterableAggregate.php Outdated Show resolved Hide resolved
Copy link
Contributor

@drupol drupol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code LGTM, nothing to add any more. So sad we had to add so many things when it can be replaced with a single line and recursion... but this is how it is in PHP.

The only missing thing is the documentation in the README file, if you could add an entry in there, it would be really nice.

@bobkorinek
Copy link
Contributor Author

The code LGTM, nothing to add any more.

I am glad I could help.

So sad we had to add so many things when it can be replaced with a single line and recursion... but this is how it is in PHP.

Yeah but that's why we have encapsulation:) You made a good point about performance.

The only missing thing is the documentation in the README file, if you could add an entry in there, it would be really nice.

I am on it.

Thank you for the nice review BTW!

@bobkorinek bobkorinek requested a review from drupol September 26, 2024 11:26
@drupol drupol merged commit 74f7aa2 into loophp:main Sep 26, 2024
13 of 14 checks passed
@drupol
Copy link
Contributor

drupol commented Sep 26, 2024

Thank you <3 !

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

Successfully merging this pull request may close these issues.

2 participants