Skip to content

Add stream name in handler context - #219

Merged
codeliner merged 4 commits into
developfrom
stream_name_in_handler_context
Nov 23, 2016
Merged

Add stream name in handler context#219
codeliner merged 4 commits into
developfrom
stream_name_in_handler_context

Conversation

@prolic

@prolic prolic commented Nov 21, 2016

Copy link
Copy Markdown
Member

No description provided.

}

$handler = $this->handlers[$event->messageName()];
$handler = Closure::bind($handler, $this->createHandlerContext($streamName));

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Any ideas an how to bind earlier, so no new context is created for every event?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Or is this no performance problem at all?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Perhaps we add stream name as third argument to the handler, but this is something I like to avoid.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

handler context could be created outside of the foreach and with a setStreamName method but I would only do this if it has really positive influence on the performance.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ah forget my idea. Immutability is king.

@codeliner codeliner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@prolic Do you want to run a benchmark? I don't think it makes a difference at least for "normal" usage but a benchmark would tell us more.

}

$handler = $this->handlers[$event->messageName()];
$handler = Closure::bind($handler, $this->createHandlerContext($streamName));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

handler context could be created outside of the foreach and with a setStreamName method but I would only do this if it has really positive influence on the performance.

@codeliner
codeliner merged commit a376656 into develop Nov 23, 2016
@prolic
prolic deleted the stream_name_in_handler_context branch November 27, 2016 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants