Skip to content

Refactor keys() and values() internals #13

Closed
@vweevers

Description

@vweevers

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, AbstractKeyIterator and AbstractValueIterator. 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:

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

No one assigned

    Labels

    semver-majorChanges that break backward compatibility

    Type

    No type

    Projects

    Status

    Done

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions