Added setMany() functionality #74
Conversation
| }); | ||
| } | ||
|
|
||
| if (entries.length > 0) { |
There was a problem hiding this comment.
I think we can remove a lot of boilerplate by reusing async's parallel function (http://caolan.github.io/async/docs.html#parallel), or even parallelLimit, so we only have X amount of writes at any given time.
There was a problem hiding this comment.
I replied on #73 about using async, but the if statement is not necessary at all, it is just to ensure that in the event someone does storage.setMany({}) that an error will not be thrown. I've never used async.parallel so I'm unsure if would be able to handle an event in which no keys are passed to the function.
There was a problem hiding this comment.
I'm unsure if would be able to handle an event in which no keys are passed to the function.
Fair enough. The current approach looks good then :)
| const value = entry.value; | ||
|
|
||
| exports.set(key, value, function(error) { | ||
| if (error) { |
There was a problem hiding this comment.
Would it be good if instead of simply returning if things were successful or not, we would return the list of properties that failed (or the ones that succeeded), so in the case of an error, the user knows which ones to retry?
There was a problem hiding this comment.
That was originally what I had planned. With the way I ended up doing it, the user will still know which ones to retry (it will set the keys one by one, and every time there is an error it will invoke the callback which is why the callback takes a second parameter finished to let the user know when all the keys have been set).
It's up to you. If we did it your way, would we simply return a list of failed keys (['foo', 'bar']) or the errors that they threw too ({ foo: error1, bar: error2 })?
There was a problem hiding this comment.
I see. I think we should return a list of failed keys along with their respective errors, and only call the final callback once.
2 participants
The PR included both the core method, the tests and an updated README.