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

Pass any variables between stages #45

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

roquie
Copy link

@roquie roquie commented Jun 13, 2018

Work example:

require __DIR__ . '/vendor/autoload.php';

use League\Pipeline\Pipeline;

$pipeline = (new Pipeline)
    ->pipe(function ($payload, $dto) {
        $dto['ten'] = 10;
        return $payload * 2;
    })
    ->pipe(function ($payload, $dto) {
        return $payload + $dto['ten'];
    });

echo $pipeline->process(5, new ArrayObject());
// 20

Also I added docblocks for better IDE autocomplete. Full backward compatibility.

*
* @param callable $stage
* @return \League\Pipeline\PipelineInterface|$this
*/
Copy link
Member

Choose a reason for hiding this comment

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

Not needed, PipelineInterface already provides all the docblocks. Same for other docblocks added in this PR.

Copy link
Author

@roquie roquie Jun 17, 2018

Choose a reason for hiding this comment

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

For others -- ok. But for this docblock for PipelineInterface returns only interface without $this. IDE autocompletion works only within interface scope and not touch process method which should be there.

{
use ParametersTrait;

public function process($payload, callable ...$stages)
Copy link
Member

Choose a reason for hiding this comment

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

Hmm I forgot about this signature, makes it significantly trickier to handle. It's pretty straight forward for the Pipeline:

function process(...$parameters) { ... }

But the processor is different because it receives the stages:

function process($payload, callable ...$stages) { ... }

$this->processor->setParameters(...$params);
return $this->processor->process($payload, ...$this->stages);
}

return $this->processor->process($payload, ...$this->stages);
Copy link
Member

Choose a reason for hiding this comment

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

It might make more sense for this to be:

return $this->processor->withParameters($params)->process(...$this->stages);

Doing so will change the signatures significantly though, and cause a major version bump.

Let me think about this for a couple of days.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, your variant is more elegant, but needs to bump the major version.

Let me think about this for a couple of days.

ok.

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