-
-
Notifications
You must be signed in to change notification settings - Fork 8
Description
Follow-up for #12, to be tackled in a next major version. As written there:
Adds a lot of new code, with unfortunately some duplicate code because I wanted to avoid mixins and other forms of complexity, which means key and value iterators use classes that are separate from preexisting iterators. For example, a method like
_seek()must now be implemented on three classes:AbstractIterator,AbstractKeyIteratorandAbstractValueIterator. This (small?) problem extends to implementations and their subclasses, if they choose to override key and value iterators to improve performance.
To come up with a more DRY approach, it may help to first reduce the differences between the 3 iterators. Mainly: change the callback signature of AbstractIterator#next() from (err, key, value) to (err, entry). Perhaps (dare I say) remove callbacks altogether.
If we can then merge the 3 classes into one, or at least have a shared and reusable base class, then unit tests can probably be simplified too, not having to repeat like so:
abstract-level/test/async-iterator-test.js
Lines 23 to 37 in 7113ad1
| for (const mode of ['iterator', 'keys', 'values']) { | |
| test(`for await...of ${mode}()`, async function (t) { | |
| t.plan(1) | |
| const it = db[mode]({ keyEncoding: 'utf8', valueEncoding: 'utf8' }) | |
| const output = [] | |
| for await (const item of it) { | |
| output.push(item) | |
| } | |
| t.same(output, input.map(({ key, value }) => { | |
| return mode === 'iterator' ? [key, value] : mode === 'keys' ? key : value | |
| })) | |
| }) |
Lastly (unrelated but I postponed it because of the next() callback signature and to avoid more breaking changes) perhaps rename iterator() to entries().
Metadata
Metadata
Assignees
Labels
Type
Projects
Status