fogine / couchbase-odm

CouchbaseODM is a promise-based Node.js ODM for Couchbase
https://fogine.github.io/couchbase-odm
MIT License
12 stars 3 forks source link

getMulti method tweak #2

Closed jurajsimbs closed 7 years ago

jurajsimbs commented 8 years ago

it would be cool if getMulti could return an array instead of object - or collection of objects. I'd like to order or filter it before passing data to next processing layer.

fogine commented 8 years ago

Hello, as I think about it, the best way to implement this would be to let the getMulti method return an object with toList, toCollection methods. However, this would break the compatibility. So for now, I'll add new option asColletion to the method which will return an array instead of an object.

Thanks for the feedback.

jurajsimbs commented 8 years ago

...not sure about that

to let the getMulti method return an object with toList, toCollection methods.

if this extra layer will be there only to provide that two methods - seems to me like extra complexity with no additional value. to me method getMulti with for example indexed param sounds less complex. then getMulti(indexed=false) will return an array (non-indexed collection) and getMulti(indexed=true) will return an object (indexed collection - by document id). It is not that important to have access to both list and collection interpretation of data at the same time - in my opinion.

fogine commented 8 years ago

if this extra layer will be there only to provide that two methods - seems to me like extra complexity with no additional value.

Well, additional value it would introduce is access to objects which implement PromiseInspection interface before they are checked for errors/processed in other ways. This way, an user would has option to process data himself, without iterating over data collection, again. This could come handy in more complex cases.

In addition, it would bring cleaner API. To me, having an option which changes format of data the method respond with, is not clean way how to implement this.

if I implement this, it will be next major version. indexed=false as the option sounds more reasonable than asCollation, thanks.

fogine commented 8 years ago

@jurajsimbs It's available on the develop branch if interested.

use case example:

    User.getMulti([
        'abeda67e-47ad-4538-ba45-860fbd5107cb',
        'b603fe68-fe3b-49d4-a4b1-bb3e16c72c93'
    ], {indexed: false}).then...

The API documentation will be available once the feature is merged to the master with v1.1.0 release.

jurajsimbs commented 8 years ago

working nicely :+1: