markrogoyski / itertools-php

PHP Iteration Tools Library
MIT License
140 stars 11 forks source link

Chain namespace #9

Closed Smoren closed 1 year ago

Smoren commented 1 year ago

Hi @markrogoyski

What do you think about new namespace with functionality of chain calls of another namespaces' methods?

For example:

$input = [1, -1, 2, -2, 3, -3];

$iter = Chain::create($input)
    ->filterTrue(static function ($value) {
        return $value > 0;
    })
    ->pairwise();

$output = iterator_to_array($iter);
// [[1, 2], [2, 3]]

This is only a draft of PR. If you think this is a good idea I'm ready to implement all the methods and cover them by tests.

coveralls commented 1 year ago

Pull Request Test Coverage Report for Build 3887127941

Warning: This coverage report may be inaccurate.

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.

Details


Totals Coverage Status
Change from base Build 3880933404: -5.7%
Covered Lines: 194
Relevant Lines: 207

💛 - Coveralls
Smoren commented 1 year ago

Hi @markrogoyski

I've added all the relevant methods, but they haven't been fully tested yet.

What do you think about this functionality?

markrogoyski commented 1 year ago

Hi @Smoren,

Sorry for the delay in responding. I don't have regular access to my PC and internet temporarily.

Thanks for the PR. I like the idea here.

A couple initial thoughts. I think a better name for this would be Stream. So like:

Stream::of($data)->...

Also, it probably makes sense to think about starting a stream from an infinite iterator, for example:

Stream::count(1, 1)->...

And similarly for random iteration as a starting point.

Anyways, cool stuff. Apologies that it may still be a few days before I can thoroughly review the PR and leave more detailed feedback.

Thanks, Mark

Smoren commented 1 year ago

@markrogoyski

Thanks for the feedback!

I am very glad that you liked this idea.

Made suggested changes to the naming.

  1. Namespace Chain renamed to Stream.
  2. Method Stream::create() renamed to Stream::of().
markrogoyski commented 1 year ago

Hi @Smoren,

I suggest adding this function to the Stream class.

    public function toArray(): array
    {
        $result = [];
        foreach ($this->iterable as $item) {
            $result[] = $item;
        }

        return $result;
    }

It would be a finalizer method on the stream, like this:

$result = Stream::of([1, 2, 3, 4, 5, 6, 7, 8, 9])
    ->filterTrue(fn ($x) => $x % 2 === 0)
    ->pairwise()
    ->toArray();

I think this will simplify your unit tests, as you will no longer need to have a foreach loop in every unit test.

I have some other ideas for methods to add, but this PR is big enough, so we can add them in later on.

Mark

Smoren commented 1 year ago

Hi @markrogoyski

  1. Tests: all the anonymous functions are replaced by arrow functions.
  2. Method toArray() is added and used in tests.
  3. Method toAssociativeArray() is added (we need it using key-value iterators, not value only) and used in tests.
markrogoyski commented 1 year ago

Hi @Smoren,

Thanks again for this great PR. I've merged it in. I'm going to continue developing the Streaming concept to add more features to it.

BTW, I removed toAssociativeArray() because it was not safe as written if zip is used, as the keys will themselves be an array, and trying to use an array as an associative array key will result in a TypeError. I will add a more flexible termination function to map custom keys via an anonymous function.

Mark

Smoren commented 1 year ago

Thank you Mark!