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
Add stream matchers. #532
Add stream matchers. #532
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple small things then 👍
/// If any matchers match the stream, no errors from other matchers are thrown. | ||
/// If no matchers match and multiple matchers threw errors, the first error is | ||
/// re-thrown. | ||
StreamMatcher emitsAnyOf(Iterable matchers) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error if the iterable is empty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
try { | ||
result = await streamMatchers[i].matchQueue(copy); | ||
} catch (error, stackTrace) { | ||
if (error != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
error
should never be null. Is this case needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, this was supposed to check firstError
.
} | ||
|
||
/// Returns a [StreamMatcher] that matches any number of | ||
/// events followed by events that match [matcher]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: The wrapping here looks arbitrary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
while (await queue.hasNext) { | ||
if (await tryHere()) return null; | ||
await queue.next; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is quadratic, right? Or I guess, more specifically O(mn) where m is the stream length and n is the matcher's length.
That's OK, but it would probably be good to mention it since it can get slow with long streams and matchers.
Or you could adapt Boyer-Moore to this. 😛
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The quadradic behavior is only relevant when the matcher can match long strings of events and when in practice it matches many long prefixes without satisfying the entire matcher. I don't think that's likely enough to come up in practice to warrant documenting.
Late to the party, but yay! Nice work @nex3! |
No description provided.