feat(sequenceEqual): adds sequenceEqual operator #1883
Conversation
| this.destination.error(errorObject.e); | ||
| } | ||
| } else { | ||
| areEqual = a === b; |
There was a problem hiding this comment.
what about making subscriber's ctor provides default value of (defaultComparor) instead of line 14, then remove this branch to check comparor exists? it'll makes comparison in checkValues and _complete lot similar, may can be extracted into common function.
There was a problem hiding this comment.
conditionals are cheaper than function calls. I originally had what you suggested, but optimized.
There was a problem hiding this comment.
Got it. please allow me one following question, per line 66 it always supplies comparor to default value, so usual consumer directly uses operator won't hit this cases unless explicitly clears it like sequenceEqual(other, null) - is that correct?
There was a problem hiding this comment.
Actually, in my most recent commit, I've removed the value checking from _complete entirely because it wasn't necessary, and also removed the defaultComparor.
There was a problem hiding this comment.
ah, I can see it now. Thanks for clarification 👍
- adds most basic tests - documentation still required - more tests necessary resolves #1882
- no marble diagram included yet
- improves code coverage - removes value checking and simplifies the comparison of the buffers, they should both be zero at that point - removes defaultComparor that should not have been used
|
Change looks good to me, |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
resolves #1882