Understanding best practices for handling returned pointers in GoLang and error checking-CodePudding

Let's say I have the following function

func GetNumber() (*int, error) {
   numb, err := getNumberFromDB(db)
   if err != nil {
       return nil, errors.New("error from db")
   }
   return numb, nil
}

numb is an *int, but the only situation in which it ever gets returned as nil, is if there is an error

So if I'm using GetNumber(), should I also be checking to see if int is nil prior to using it? It feels like I should to prevent future changes to GetNumber from causing issues (or maybe getNumberFromDB starts returning no error for some reason at some point).

But I also feel like it's very redundant and I'm adding these conditional checks everywhere. Just want to make sure I'm not following best practices.

IE:

numb, err : = GetNumber()
if err != nil {
    //handle error
}
if numb == nil {
   // this shouldn't happen
}

Another Example:

func EmailFromContext(ctx context.Context, key interface{}) (*string, error) {
    ctxVal, ok := ctx.Value(key).(*validator.ValidatedClaims)
    if !ok {
        // so should this be returning "" instead of nil?
        return nil, errors.New("could not convert claims")
    }
    return &claims.Email, nil
}

CodePudding user response：

Not knowing the implementation of getNumberFromDB(), or the underlying schema, it might be possible that a record was found, but the field in the table had a NULL value. In that case, it's possible for getNumberFromDB to return nil, nil (no number, no error). In that case, you'd still need to check for nil, even if no error was returned. I would, however, move that check to the GetNumber function, and not return pointers to integers. They're a PITA most of the time, especially when dealing with concurrency. I would write something like this:

func GetNumber() (int, error) {
    i, err := getNumberFromDB()
    if err != nil {
        return 0, err // perhaps wrap the DB error
    }
    if i == nil {
        return 0, ErrNumberNotSet // pre-defined error variable
    }
    return *i, nil // return value
}

With this function, you can just do:

i, err := GetNumber()
if err != nil {
    // handle error
}
fmt.Sprintf("Got number %d from DB\n", i)

Furthermore, you can change the GetNumber function a bit more to set a default value for NULL values:

func GetNumber(def int) (int, error) {
    i, err := getNumberFromDB()
    if err != nil {
        return 0, err // perhaps wrap the DB error
    }
    if i == nil {
        return def, nil
    }
    return *i, nil // return value
}

In which case:

num, err := GetNumber(123)
if err != nil {
    // handle DB error
}
fmt.Printf("Number is: %d\n", num)

CodePudding user response：

In general, a non-nil error means the other results are meaningless. However, this is not always strictly followed by everyone. For example, if you're writing a gRPC server, you are not expected to return a non-nil value with a non-nil error. Whereas in the stdlib, an io.Reader returns the number of bytes read and io.EOF.

So there are several options, all of which would work:

You can assume nil error means meaningful result. In this case you have to define what meaningful result is. Based on your description, it looks like it cannot be nil, so it is OK not checking for nil.

If the function cannot return nil, then it may make sense to not return a pointer, unless there is a reason for it to be a pointer.

But then, in many situations, it may make sense to return nil,nil.

My suggestion would be to return (int,error), unless there is another reason to return a pointer. This would simplify the use of the function for the caller.