-2
\$\begingroup\$

I have this following code snippet, I was wondering If I am using unnecessary If statements?

func ReadBook(b *book) (string, error) {
    bookProps, err := GetFunc1(b.name)
    if err != nil {
        if awsErr, ok := err.(awserr.Error); ok {
            if awsErr.Code() == ResourceNotFoundException {
                _, err = Create(b.name)
                if err != nil {
                    return nil, err
                }
                bookProps, err = GetFunc1(b.name)
                if err != nil {
                    return nil, err
                }
            } else {
                return nil, err
            }
        } else {
            return nil, err
        }
    }
    return bookProps.name, nil
}
\$\endgroup\$
2
\$\begingroup\$

Yes. It's not an absolute rule but generally if you're using more than one if err != nil per function or more then one level of error checking in Go you should refactor.

I can't test this because you didn't provide enough code to use it but here is first pass flattened version that reads more clearly:

func ReadBook(b *book) (string, error) {
    
    bookProps, err := GetFunc1(b.name)
    if err == nil {
        return bookProps.name, nil
    }

    awsErrCode, err := unpackAWSErr(err)
    if err != nil {
        return nil, err
    }

    // Could easily be replaced with switch{ case: } to handle different error codes
    if awsErrCode == ResourceNotFoundException {
        _, err = Create(b.name)
        if err != nil {
            return err
        }
    }

    bookProps, err := GetFunc1(b.name)
    if err == nil {
        return bookProps.name, nil
    }

    return nil, err
}

// assuming awsErr.Code() returns an int
func unpackAWSErr (err error) (int, error) {
    if awsErr, ok := err.(awserr.Error); ok {
        return awsErr.Code(), nil
    }

    return (-1, ok)
}

Also, I'm guessing that at some point you're going to want either rewrite your function or create a wrapper to accept a func(string) (string, error) in place of GetFunc1(). Right now that's an unmoored function floating through your code without any direct relation to the book type or being passed explicitly into ReadBook()

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.