Skip to content
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

feat: map method with default _map fallback #376

Closed
wants to merge 1 commit into from

Conversation

MeirionHughes
Copy link
Member

@MeirionHughes MeirionHughes commented May 6, 2021

As per Level/community#100

Adds new map method to serve as a "multi-get" / "batch-get" method for implementing 'downs.

Adds protected overload-able _map method with default fallback to looping over each key and using _get. i.e. non-breaking for non-implementing 'downs.

@MeirionHughes MeirionHughes requested review from vweevers and removed request for vweevers May 6, 2021 14:54
@MeirionHughes
Copy link
Member Author

this needs more tests to confirm _map is working properly

@MeirionHughes
Copy link
Member Author

seems not all tests are ran within abstract-down's test script - I've added a test into self.js now to check the default loop-get fallback. Not sure how to make sure the export.map tests are okay though.

@vweevers vweevers added the semver-minor New features that are backward compatible label May 7, 2021
@@ -427,6 +435,14 @@ Delete an entry. There are no default options but `options` will always be an ob

The default `_del()` invokes `callback` on a next tick. It must be overridden.

### `db._map(keys, options, callback)`

Map each `key` in the `keys` array to a `value`. The `options` object will always have the following properties: `asBuffer`. If _any_ key in `keys` does not exist, call the `callback` function with a `new Error('NotFound')`. Otherwise call `callback` with `null` as the first argument and the values array as the second.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If any key in keys does not exist, call the callback function with a new Error('NotFound')

Not sure this is desirable. You wouldn't know which key wasn't found. With parallel writes, it's not unlikely that keys will contain deleted keys.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll remove the default fallback - but how would you want to deal with this? if it fails on the n'th item, then return an array with n-1 items and a non-null error?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking that we could yield an undefined value. For example, given keys ['a', 'b', 'c'] and key b does not exist, the result would be [value, undefined, value].

throw new Error('map() requires a callback argument')
}

if (Array.isArray(keys) === false || keys === null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (Array.isArray(keys) === false || keys === null) {
if (!Array.isArray(keys)) {

The null-check is redundant.


const _keys = new Array(keys.length)

for (const index in keys) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const result = new Array(count)
let index = 0

if (count === 0) return this._nextTick(callback, null, [])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do this in map() instead?

@vweevers
Copy link
Member

Superseded by #381

@vweevers vweevers closed this Sep 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-minor New features that are backward compatible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants