marcello3d / node-mongolian

[project inactive] Mongolian DeadBeef is an awesome Mongo DB driver for node.js
https://groups.google.com/group/node-mongolian
zlib License
349 stars 50 forks source link

collection.find() expected results, api change #50

Open JonDum opened 13 years ago

JonDum commented 13 years ago

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?

marcello3d commented 13 years ago

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. :-)

JonDum commented 13 years ago

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

qharlie commented 12 years ago

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

marcello3d commented 12 years ago

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).

qharlie commented 12 years ago

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 ?

marcello3d commented 12 years ago

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.