WIP: V3 Sync Unions #65
Conversation
|
Note: the failing test is because |
jacoscaz
left a comment
jacoscaz
left a comment
There was a problem hiding this comment.
Nicely done, I like how the maxBufferSize is used to limit the number of source the iterator reads from. These are just minor nitpicks.
| prepend(items: T[] | AsyncIterator<T>): AsyncIterator<T> { | ||
| return this.transform({ prepend: items }); | ||
| return new UnionIterator( | ||
| [Array.isArray(items) ? new ArrayIterator(items, { autoStart: false }) : items, this], |
There was a problem hiding this comment.
After #63 is merged, this could become simply wrap(items, { letIteratorThrough: true }) (thus supporting a wider range of types for items).
| append(items: T[] | AsyncIterator<T>): AsyncIterator<T> { | ||
| return this.transform({ append: items }); | ||
| return new UnionIterator( | ||
| [this, Array.isArray(items) ? new ArrayIterator(items, { autoStart: false }) : items], |
| const sources = this._pending!.sources!; | ||
| delete this._pending!.sources; | ||
| // @ts-ignore | ||
| this._sources = (Array.isArray(sources) ? fromArray(sources) : wrap(sources)).map<AsyncIterator<T>>((it: any) => isPromise(it) ? wrap(it as any) : (it as AsyncIterator<T>)); |
There was a problem hiding this comment.
I think this could also be simplified after #63
| private _currentSource = -1; | ||
| export class UnionIterator<T> extends AsyncIterator<T> { | ||
| private _sources : AsyncIterator<AsyncIterator<T>>; | ||
| private buffer = new LinkedList<AsyncIterator<T>>(); |
There was a problem hiding this comment.
I think this needs a _ prefix.
| mutateFilter(filter: (item: V) => boolean) { | ||
| let last: LinkedNode<V> | null; | ||
| let next: LinkedNode<V> | null; | ||
| while (this._head !== null && !filter(this._head.value)) { |
There was a problem hiding this comment.
I suspect this could be simplified into one single loop if we were to have the list itself implement the LinkedNode interface, referencing the first node in the chain as next instead of _head. It might not be worth the effort, though.
|
Thanks for the review - I'll wait for #63 to be merged and the rebase and clean this up :) |
| private _sources : InternalSource<T>[] = []; | ||
| private _pending? : { sources?: AsyncIterator<MaybePromise<AsyncIterator<T>>> }; | ||
| private _currentSource = -1; | ||
| export class UnionIterator<T> extends AsyncIterator<T> { |
There was a problem hiding this comment.
Document whether or not it is in-order; crucial to prepend and append.
|
Did work getting this up to date on the train. Will push changes later today |
Same idea as #54 but targeted for v3.x.
#63 and #59 should be merged first
Now also resolves #67