node-mongolian icon indicating copy to clipboard operation
node-mongolian copied to clipboard

collection.find() expected results, api change

Open JonDum opened this issue 13 years ago • 6 comments

In the mongo shell, db.collection.find() returns all documents, however, in mongolian, the same command will return a cursor. What's with the discrepancy? Shouldn't just running db.collection.find(query, callback) give you all documents in that collection that match the query?

JonDum avatar Aug 25 '11 21:08 JonDum

Currently, since Node.js is asynchronous combined with being able to string together things like sort, limit, skip, I currently require a helper method on the cursor to get the data:

cursor.toArray(callback) // pulls all the data down from the server for this cursor into a single Array before triggering the callback
cursor.forEach(func, callback) //  calls func for each document, and callback upon completion or error
cursor.next([callback]) // sends the next document or null to the callback if there are no more

It might make sense to add an optional callback parameter to find(...) that serves as a shortcut for find(...).toArray(callback). I'll keep this open until I decide if that's clean and consistent. :-)

marcello3d avatar Aug 27 '11 03:08 marcello3d

Sounds good, mate. Just my two sense on the API.

JonDum avatar Sep 02 '11 21:09 JonDum

I second this - though I would vote for a new series of functions that return the array instead of the cursor.

qharlie avatar Dec 27 '11 18:12 qharlie

Is it so onerous to type .toArray()?

My fear is that it makes the sort/limit/skip chains a little weird (and creates two ways of doing the same thing):

collection.find({age:{$gt:18}).sort({name:1}).limit(20).skip(5)

Where should the array callback be in that example? In the skip function?

collection.find({age:{$gt:18}).sort({name:1}).limit(20).skip(5, function(error, array) { ... })

I think this is clearer:

collection.find({age:{$gt:18}).sort({name:1}).limit(20).skip(5).toArray(function(error, array) { ... })

Less important, but worth noting, it also encourages less efficient coding (toArray uses more memory in large results than forEach, since it has to buffer all the results first, for example).

marcello3d avatar Dec 30 '11 19:12 marcello3d

Good point , I hadn't that about the chaining. I agree that having an extra parameter for each chained function is hard to read and would be confusing.

I will say that I've forgotten to call toArray() quite a few times, when just using find() , so maybe a shortcut is the best approach.

Is there anyway at all to have a non-asynch call to find(), that simply returns the entire result set ?

qharlie avatar Dec 30 '11 19:12 qharlie

That's good to know. How big of a result set are you using in these situations? Could you describe some scenarios where you want multiple results but aren't using any chained operators?

As for as synchronous calls, that's not possible with node.js, since all the network IO is asynchronous.

marcello3d avatar Dec 30 '11 20:12 marcello3d