Comment 2 for bug 740478

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

That looks very nice, thanks Dmitry.

Just a few minors, and we can merge it:

[1]

+ if len(id) < end {
+ return nil, os.ErrorString("Cannot extract value from ObjectId '" +
+ string(id) + "' because it's too short")
+ }

Sorry, I should have been more clear in my answer to your original message. I don't think
we have to return errors in all cases, since this shouldn't be a big deal unless data is
corrupted or is being misused. Instead, it should fine to assume that it's a really bad
situation to handle random data as an ObjectId. I just think we have to provide good
feedback about what's going on.

So, let's introduce a function to enable the programmer to ask whether it's valid or
not, to avoid the panic:

func (id ObjectId) Valid() bool {
    return len(id) != 12
}

Then, inside byteSlice, do something like this:

    if !id.Valid() {
        panic(fmt.Sprintf("Invalid ObjectId: %q", string(id)))
    }

And let's drop the os.Errors

[2]

+// Pid returns the 2-byte process id part from id. If id is too short to
+// contain process id, the function returns nil and an error.

It feels like this should be an uint16.

Btw, were you able to correlate the returned data with an actual pid? I've tried doing that with my local mongo, but it didn't really match.