Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Idiomatic way to return (potentially shadowed) variables which have been defined as part of a separate multiple variable return

Normal Situation

When writing a function with a named return value, you can typically use naked returns (whether or not you should is a separate discussion). They might look something like the following:

func add(x, y int) (z int) {
    z = x + y
    return
}

return here meaning the same as return z

Problematic Situation

However with the abridged snippet below...

func loadModule(moduleName, fileRoot string) (module []byte) {
    if strings.HasSuffix(moduleName, ".md") {
        module, err := readConvertMDFile(fileRoot + "htdocs/md/" + moduleName)
        if err != nil {
            log.Println(err)
        }
        return module
    } else {
        module = []byte{}
        return
    }
}

(This snippet runs fine, and is my current solution to the problem)

... the compiler will complain that module is shadowed if instead of return module there is just return. This is because module was declared (a second time) along with err, which has to be declared as it doesn't exist yet in this scope.

Possible Solutions

  1. Do as I have done and explicitly name the return variable. While this isn't a terrible solution, I feel as though there should be a way to arrange the code so that it runs as it should with a naked return. Others have commented that this explicit return leads to 'code smell'.

  2. Add a var err error at the beginning and use a multiple assignment rather than declaration. Probably a better solution, but I would prefer to use implicit assignment where possible for the sake of consistency and to reduce unnecessary lines.

  3. Use a temporary moduleT variable then assign module = moduleT... this just feels messy and redundant.

While I can get the compiled result I'm looking for, I'm hoping someone can suggest a clear, idiomatic way of writing this.

like image 829
azmr Avatar asked Apr 23 '14 21:04

azmr


1 Answers

I always use your suggested solution 2 - add an extra var statement.

func loadModule(moduleName, fileRoot string) (module []byte) {
    var err error
    if strings.HasSuffix(moduleName, ".md") {
        module, err = readConvertMDFile(fileRoot + "htdocs/md/" + moduleName)
        if err != nil {
            log.Println(err)
        }
        return
    } else {
        // no need for this as module will be nil if it isn't written to
        // a nil slice is perfectly legal and has len()=0
        // and can be appended to etc
        // module = []byte{}
        return
    }
}

Option 2 is the most efficient solution too. Remember that go returns all values on the stack so a named return value is equivalent to a stack allocated variable.

If in option 1 or option 3 you don't have a naked return, then there is an implicit module = module or module = moduleT statement in there anyway.

Unfortunately variable shadowing is something which bites every Go programmer after a while. I'd quite like the compiler to disallow all shadowing within a function as it is a source of real bugs.

like image 157
Nick Craig-Wood Avatar answered Oct 20 '22 06:10

Nick Craig-Wood