将作为单独多变量返回的(可能被遮蔽的)变量以惯用方式返回。

huangapple go评论87阅读模式
英文:

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

问题

正常情况

当编写一个带有命名返回值的函数时,通常可以使用裸返回(是否应该使用是另外讨论的问题)。它们可能看起来像下面这样:

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

这里的return的意思与return z相同

存在问题的情况

然而,在下面的简化代码片段中...

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
    }
}

(这个代码片段可以正常运行,是我目前解决这个问题的方法)

...如果return module改为return,编译器会报错*module is shadowed*。这是因为moduleerr一起被声明了(第二次声明),而err必须被声明,因为在这个作用域中它还不存在。

可能的解决方案

  1. 像我所做的那样,显式地给返回变量命名。虽然这不是一个糟糕的解决方案,但我觉得应该有一种方法来安排代码,使其能够使用裸返回运行。[其他人评论][1]说这种显式返回会导致“代码异味”。

  2. 在开头添加var err error,使用多重赋值而不是声明。这可能是一个更好的解决方案,但出于一致性和减少不必要的行数的考虑,我更喜欢在可能的情况下使用隐式赋值。

  3. 使用临时变量moduleT,然后将module = moduleT赋值...这种方法看起来很混乱和多余。

虽然我可以得到我想要的编译结果,但我希望有人能提出一种清晰、惯用的编写方式。

英文:

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][1] 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.

[1]: https://plus.google.com/+WilliamKennedy/posts/8hMjHhmyNk2 "Multiple viewpoints"

答案1

得分: 7

我总是使用你建议的解决方案2-添加一个额外的var语句。

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 {
        // 不需要这个,因为如果没有写入module,它将为nil
        // 一个nil切片是完全合法的,其长度为0
        // 并且可以进行追加等操作
        // module = []byte{}
        return
    }
}

选项2也是最高效的解决方案。请记住,Go将所有值都返回到堆栈上,因此命名返回值等效于堆栈分配的变量。

如果在选项1或选项3中没有裸返回,那么在其中肯定有一个隐式的module = modulemodule = moduleT语句。

不幸的是,变量遮蔽是每个Go程序员在一段时间后都会遇到的问题。我很希望编译器在函数内部禁止所有的遮蔽,因为它是真正的bug来源。

英文:

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.

答案2

得分: 0

在我写这个问题的时候,我的函数看起来像下面这样:

(主要展示其冗长性)

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 if strings.HasSuffix(moduleName, ".html") {
        module, err := ioutil.ReadFile(fileRoot + "htdocs/html/" + moduleName)
        if err != nil {
            log.Println(err)
        }
        return module
    } else if strings.HasSuffix(moduleName, ".js") {
        module, err := ioutil.ReadFile(fileRoot + "htdocs/js/" + moduleName)
        if err != nil {
            log.Println(err)
        }
        return module
    } else if strings.HasSuffix(moduleName, ".css") {
        module, err := ioutil.ReadFile(fileRoot + "htdocs/css/" + moduleName)
        if err != nil {
            log.Println(err)
        }
        return module
    } else {
        module = []byte{}
        return
    }
}

这是使用我提出的解决方案1。它有很多重复的代码(我还是一个初学者)。如果我改用建议的解决方案2(但不是我最初想到的方式),在函数顶部加上var err error,代码将改进两个方面:

func loadModule(moduleName, fileRoot string) (module []byte) {
    var err error
    switch {
    case strings.HasSuffix(moduleName, ".md"):
        module, err = readConvertMDFile(fileRoot + "htdocs/md/" + moduleName)
    case strings.HasSuffix(moduleName, ".html"):
        module, err = ioutil.ReadFile(fileRoot + "htdocs/html/" + moduleName)
    case strings.HasSuffix(moduleName, ".js"):
        module, err = ioutil.ReadFile(fileRoot + "htdocs/js/" + moduleName)
    case strings.HasSuffix(moduleName, ".css"):
        module, err = ioutil.ReadFile(fileRoot + "htdocs/css/" + moduleName)
    default:
        module = []byte{}
    }
    if err != nil {
        log.Println(err)
    }
    return
}

现在不再有重名变量,错误日志记录和返回语句可以移到每个if语句之外,从而使代码更清晰。

可能还有改进的方法。编辑:...并且有了,根据@ANisus的建议,if-else链已被替换为switch语句。

英文:

At the time I wrote the question, my function looked like the following:

(shown mostly to demonstrate its verbosity)

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 if strings.HasSuffix(moduleName, ".html") {
    	module, err := ioutil.ReadFile(fileRoot + "htdocs/html/" + moduleName)
    	if err != nil {
    		log.Println(err)
    	}
    	return module
    } else if strings.HasSuffix(moduleName, ".js") {
    	module, err := ioutil.ReadFile(fileRoot + "htdocs/js/" + moduleName)
    	if err != nil {
    		log.Println(err)
    	}
    	return module
	} else if strings.HasSuffix(moduleName, ".css") {
		module, err := ioutil.ReadFile(fileRoot + "htdocs/css/" + moduleName)
		if err != nil {
			log.Println(err)
		}
 		return module
    } else {
    	module = []byte{}
    	return
    }
}

This uses my suggested solution 1. It has a lot of repeated code (I'm still something of a beginner). If I instead use suggested solution 2 (but not in the way I originally thought of it) by putting var err error at the top of then function, the code is improved in two ways:

func loadModule(moduleName, fileRoot string) (module []byte) {
	var err error
	switch {
	case strings.HasSuffix(moduleName, ".md"):
		module, err = readConvertMDFile(fileRoot + "htdocs/md/" + moduleName)
	case strings.HasSuffix(moduleName, ".html"):
		module, err = ioutil.ReadFile(fileRoot + "htdocs/html/" + moduleName)
	case strings.HasSuffix(moduleName, ".js"):
		module, err = ioutil.ReadFile(fileRoot + "htdocs/js/" + moduleName)
	case strings.HasSuffix(moduleName, ".css"):
		module, err = ioutil.ReadFile(fileRoot + "htdocs/css/" + moduleName)
	default:
		module = []byte{}
	}
	if err != nil {
			log.Println(err)
		}
	return
}

There are no shadowed variables any more, and both the error logging and the return can be moved out of each if statement, resulting in much clearer code.

There may well be a way to improve on this. EDIT: ...and there is, following @ANisus' suggestion, the if-else chain has been replaced with a switch statement.

huangapple
  • 本文由 发表于 2014年4月24日 05:41:12
  • 转载请务必保留本文链接:https://go.coder-hub.com/23255748.html
匿名

发表评论

匿名网友

:?: :razz: :sad: :evil: :!: :smile: :oops: :grin: :eek: :shock: :???: :cool: :lol: :mad: :twisted: :roll: :wink: :idea: :arrow: :neutral: :cry: :mrgreen:

确定