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

Improvement/merged stream iterator #367

Merged
merged 9 commits into from
May 17, 2019

Conversation

basz
Copy link
Member

@basz basz commented May 13, 2019

implementation of prooph/pdo-event-store#201

some notes;

  • works like any other iterator (no array result from current with results from multiple iterators)
  • can work with any number of streams (but not zero)
  • can query for streamName of current message (needed for tracking of projection positions)
  • invalid iterators may become valid again while still iterating other valid ones without problem
  • think it should be still fastish

@coveralls
Copy link

coveralls commented May 13, 2019

Coverage Status

Coverage increased (+0.02%) to 99.031% when pulling f732532 on basz:improvement/merged-stream-iterator into a98615b on prooph:7.x.

@basz
Copy link
Member Author

basz commented May 13, 2019

think this does the trick. eager to hear what you think. bas.

@basz
Copy link
Member Author

basz commented May 14, 2019

heads up. I identified a bug.

@basz
Copy link
Member Author

basz commented May 14, 2019

@prolic is there a reason you return false instead of null as defined by the interface?

https://github.com/prooph/pdo-event-store/blob/master/src/PdoStreamIterator.php#L188
https://php.net/manual/en/iterator.key.php

I would want to define public function key(): ?int within the MergedStreamIterator and simply return the contained (pdo) iterators key() result.

@prolic
Copy link
Member

prolic commented May 17, 2019

I don't think the return value of key() is used anywhere, or is it?

@prolic prolic self-assigned this May 17, 2019
@prolic prolic merged commit 57c89a3 into prooph:7.x May 17, 2019
@basz
Copy link
Member Author

basz commented May 17, 2019

I don't think the return value of key() is used anywhere, or is it?

No it isn’t (i think)

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.

3 participants