removes nesting from recorded events - #208
Conversation
|
Can you add a test-case that is failing without this? |
|
@prolic I guess a test-case is difficult here because the test would need to reach xdebugs max nesting function level and therefor xdebug would need to be installed and set up with an appropriate limit. |
| $this->actionEventEmitter->dispatch($event); | ||
|
|
||
| $this->recordedEvents = new ArrayIterator(); | ||
| $this->resetRecordedEvents(); |
There was a problem hiding this comment.
I'm not sure if it is worth it, to create a new method for a one line of code? I would prefer $this->recorderEvents = new AppendIterator(). Every function call in PHP is a performance drawback too.
There was a problem hiding this comment.
@oqq In a domain model I would do it exactly like you: clean and readable code that tells you what it does. But @sandrokeil 's reasoning wins here, because prooph is infrastructure and performance has very high priority.
There was a problem hiding this comment.
Of course, my reason was to be consistent what "recordedEvents" should be. But i agree with the performance reasons.
|
It would be possible to implement a hackish test case, which would be skipped if xdebug is not available. But for me it is currently not possible to run xdebug with php7.1 |
|
also manually merged into master |
4 participants
This PR closes #202