"not found" error in function (q *Query) One

[mek]

Why does (q *Query) One return an error "not found" when no document is found? I don't think that reflects mongodb's behavior. Plus, (q *Query) All will return a nil when nothing is found. 

I think the function should be checking if the passed "result" is a pointer, and setting it to nil instead of returning an error if nothing is found.

I'm pretty sure it is difficult to change this at this point as people's code will break but at least the documentation should be making it clear. It seems a comparison to mgo.ErrNotFound is the way to go and this is not in the docs.

-mek

[Gustavo Niemeyer]

On Thu, Oct 31, 2013 at 5:06 PM, mek <emreka...@gmail.com> wrote:
> Why does (q *Query) One return an error "not found" when no document is
> found? I don't think that reflects mongodb's behavior. Plus, (q *Query) All
> will return a nil when nothing is found.

My goal was (and still is) to have a good API for Go, and I think the
goal was achieved. The API is internally consistent, straightforward,
and pleasant to use.

Just quickly ponder about this statement:

err := collection.Find(qdoc).One(&result)

How could you tell if the document wasn't found if err is nil? This
is a basic rule of API design, if it hasn't worked, make it obvious.

Now, consider the other statement:

err := collection.Find(qdoc).All(&slice)

The result is now a slice, and it may contain one, five, or zero
documents. In all of these cases, the slice result will contain what
you asked for: all the documents that match the query; zero isn't
special.

> I think the function should be checking if the passed "result" is a pointer,
> and setting it to nil instead of returning an error if nothing is found.

That's a bad API, as it'd both be unusual, and also kill your local
reference to the value address.

> I'm pretty sure it is difficult to change this at this point as people's

There's no reason to change it. We have a pretty good API.


gustavo @ http://niemeyer.net

[Gustavo Niemeyer]

On Thu, Oct 31, 2013 at 8:52 PM, mek <emreka...@gmail.com> wrote:
> And don't get me wrong, this is good work. I just think you made a wrong
> decision by returning an error in the "not found" case.

That's a fine perspective, but I've explained the reasons behind the
choice, and after writing a significant amount of code with that API,
I can easily tell you that it was the right decision. Sorry, but it's
here to stay.


gustavo @ http://niemeyer.net

[Tom Utley]

I hate to dig up an old issue, but I thought I'd post an idea here just in case anybody cared.

I don't think "not found" is an error. I think it is normal operation for a database. You ran a query, and the result was empty. That's not an error, it's just an empty result.

That being said I understand the reason this was done this way based on other error handling in Go.

So my idea is that there could be a function called "Exists" that works just like Find but returns a boolean value in addition to the normal operation of Find. You would use this function like so:

found, err := collection.Exists(qdoc).One(&result)

if err != nil {//do whatever error handling you need}

if !found {//oh noes we didn't find anything}