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

Fix inits operation #191

Merged
merged 8 commits into from
Sep 11, 2021
Merged

Fix inits operation #191

merged 8 commits into from
Sep 11, 2021

Conversation

drupol
Copy link
Collaborator

@drupol drupol commented Aug 26, 2021

This PR:

  • Fix the Inits operation, it works with any type of keys now.
  • It breaks backward compatibility (minor)
  • Has unit tests (phpspec)
  • Has static analysis tests (psalm, phpstan)
  • Has documentation

@drupol drupol marked this pull request as ready for review August 26, 2021 08:56
@@ -24,26 +24,26 @@ final class Inits extends AbstractOperation
/**
* @pure
*
* @return Closure(Iterator<TKey, T>): Generator<int, list<T>>
* @return Closure(Iterator<TKey, T>): Generator<int, list<array{0: TKey, 1: T}>>
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we not use Unpack to keep the previous behaviour?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's unfortunately not possible because each items is converted to an array of items.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unless using generators in place of arrays... But I don't find that very user friendly at all.

Copy link
Collaborator

@AlexandruGG AlexandruGG left a comment

Choose a reason for hiding this comment

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

Could you add an example in the docs of how a user could get the same result as previously?

E.g.

Collection::fromIterable(range('a', 'c'))
    ->inits()
    ->....()

// to get this -> [ ['a'], ['a', 'b'], ['a', 'b', 'c'] ]

@drupol
Copy link
Collaborator Author

drupol commented Aug 29, 2021

Job done. I updated the documentation examples.

Comment on lines +26 to +27
->inits()
->map(static fn (array $data): array => array_column($data, 1));
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could add another equivalent one:

Collection::fromIterable($input)
    ->inits()
    ->pluck('*.1');

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will add it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I added it 😉

@github-actions
Copy link

github-actions bot commented Sep 5, 2021

Since this pull request has not had any activity within the last 5 days, I have marked it as stale.
I will close it if no further activity occurs within the next 5 days.

@github-actions github-actions bot added the stale label Sep 5, 2021
@drupol drupol removed the stale label Sep 7, 2021
@AlexandruGG
Copy link
Collaborator

@drupol please feel free to merge this once the checks have finished 😁

@drupol drupol merged commit df51c6b into master Sep 11, 2021
@drupol drupol deleted the fix/update-inits-operation branch September 11, 2021 11:06
@drupol
Copy link
Collaborator Author

drupol commented Sep 13, 2021

Thanks @AlexandruGG :)

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

Successfully merging this pull request may close these issues.

None yet

2 participants