LLLeon / Blog

LLLeon 的部落格
15 stars 4 forks source link

Go 代码评审意见 #18

Open LLLeon opened 4 years ago

LLLeon commented 4 years ago

此文为翻译文章,这是原文地址

此页面收集在审核 Go 代码期间所做的常见注释,以便可以用缩写来指代详细的解释。这是一长串常见的错误,不是一个全面的风格指南。

你可以把这当作是 Effective Go 的补充。

Gofmt

在你的代码中运行 gofmt 来自动修复大多数机械样式的问题。几乎所有的 Go 代码都在使用 gofmt。本文档的其余部分将讨论非机械式的点。

另一种选择是使用 goimportsgofmt 的超集,它根据需要添加(和删除)导入行。

Comment Sentences

查看 https://golang.org/doc/effective_go.html#commentary。注释记录声明应该是完整的句子,即使这看起来有点多余。这种方法使它们在被提取到 godoc 文档时有良好的风格。注释应当以被描述的事物的名称开始,以句号结尾:

// Request represents a request to run a command.
type Request struct { ...

// Encode writes the JSON encoding of req to w.
func Encode(w io.Writer, req *Request) { ...

诸如此类。

Contexts

context.Context 类型的值携带安全证书,跟踪信息,截止期限,和跨 API 和进程边界的取消信号。Go 程序在整个函数调用链中显式传递 Context,从传入的 RPC 和 HTTP 请求到传出的请求。

大多数使用 Context 的函数应当以它作为它们的第一个参数:

func F(ctx context.Context, /* other arguments */) {}

一个从不特定于请求的函数可以使用 context.Background(),但即使你认为不需要,也可以用 Context 来传递 err。默认情况是传递一个 Context,如果你有足够的理由说明另一个选择是错误的,也可以直接使用 context.Background()

不要向 struct 类型添加 Context 成员,而是给该类型中每个需要传递 Context 的方法添加一个 ctx 参数。一个例外是某个方法的签名必须与标准库或第三方库中的接口匹配。

不要在函数签名中创建自定义的 Context 类型或使用 Context 以外的接口。

如果你需要传递应用程序数据,把它放到一个参数中,在 receiver 中,全局变量中,或者它真的属于这里,放到一个 Context 值中。

Context 是不可变的,因此将相同的 ctx 传递给多个分享相同的截止期限、取消信号、证书和 parent 追踪等内容的调用是没有问题的。

Copying

为避免意外的别名使用,从另一个包拷贝 struct 时要小心。例如,bytes.Buffer 类型包含 []byte 切片,并且作为小字符串的优化,一个切片可以引用的小字节数组。如果你拷贝一个 Buffer,副本中的切片可能是原始数组的别名,导致随后的方法调用产生惊人的效果。

通常,如果其方法与指针类型 * T 相关联,则不要复制类型 T 的值。

Declaring Empty Slices

当声明一个空切片时,更推荐:

var t []string

而不是:

t := []string{}

前者声明一个 nil 切片值,而后者为非 nil 值,但长度却为 0。它们在功能上是等价的 —— 它们的 lencap 都是 0 —— 但 nil 切片是首选风格。

注意,在有限的情况下,非 nil 但长度为 0 的切片是首选的,比如在编码 JSON 对象时(一个 nil 切片编码为 null,而 []string{} 编码为 JSON 数组 [])。

当设计接口时,避免区分 nil 切片和非 nil 的 0 长度切片,因为这可能导致微妙的编程错误。

更多关于 Go 中 nil 的讨论请参见 Francesc Campoy 的演讲:Understanding Nil

Crypto Rand

不要使用包 math/rand 来生成键,即使是一次性的。未加 seed 的,生成器是完全可预测的。用 time.Nanoseconds() 进行 seed,只有少量的熵。

改为使用 crypto/rand 的 Reader,并且如果你需要文字,打印为十六进制或 base64:

import (
    "crypto/rand"
    // "encoding/base64"
    // "encoding/hex"
    "fmt"
)

func Key() string {
    buf := make([]byte, 16)
    _, err := rand.Read(buf)
    if err != nil {
        panic(err)  // out of randomness, should never happen
    }
    return fmt.Sprintf("%x", buf)
    // or hex.EncodeToString(buf)
    // or base64.StdEncoding.EncodeToString(buf)
}

Doc Comments

所有顶层、导出的名称都应该有 doc 注释,重要的的非导出类型或函数声明也是如此。在 https://golang.org/doc/effective_go.html#commentary 查看有关注释约定的更多信息。

Don't Panic

查看 https://golang.org/doc/effective_go.html#errors。不要在正常的错误处理中使用 panic,使用 error 和多返回值。

Error Strings

错误字符串不应该大写(除非以专有名词或首字母缩写开头)或者以标点符号结束,因为它们通常是按照其它上下文打印的。也就是说,使用 fmt.Errorf("something bad") 而不是 fmt.Errorf("Something bad"),因此 log.Printf("Reading %s: %v", filename, err) 格式的信息中间没有假的大写字母。这不适用于日志记录,它隐式的面向行,而不是在其它消息中组合。

Examples

当添加一个新包时,包括预期使用的例子:一个可运行的例子,或一个演示完整调用序列的简单测试。

阅读更多关于 testable Example() functions

Goroutine Lifetimes

当你创建 goroutine 时,明确它们何时(或是否)退出。

goroutine 可以通过阻塞在 channel 读或者写上而发生泄漏:GC 将不会终止 goroutine,即使阻塞它的 channel 是不可达的。

即使 goroutine 没有泄漏,在它们不再被需要时而放任不管,也会导致其它微妙且不易诊断的问题。往关闭了的 channel 上发送会导致 panic。在 “不需要结果” 之后修改仍在使用的输入仍然可能导致数据竞争。 并且将 goroutines 放任不管任意长时间会导致不可预测的内存使用。

尽量保持并发代码尽量简单,使 goroutine 的生命期很明显。如果这不可行,记录 goroutine 退出的时间和原因。

Handle Errors

查看 https://golang.org/doc/effective_go.html#errors。不要使用 _ 变量丢弃错误。如果函数返回一个错误,检查它以确保函数成功。处理错误,返回错误,或者,在真正特殊的情况下,panic。

Imports

避免重命名导入,除非是为了避免名称冲突,好的包名不需要重命名。万一发生冲突,最好重命名最本地化或特定于项目的导入。

导入按组组织,它们之间留有空白行。标准库的包总是在第一组中:

package main

import (
    "fmt"
    "hash/adler32"
    "os"

    "appengine/foo"
    "appengine/user"

    "github.com/foo/bar"
    "rsc.io/goversion/version"
)

goimports 会为你做这件事。

ImportBlank

仅因为其副作用而导入的包(使用语法 import _ "pkg")应该只在程序的 main 包中导入,或者在需要它们的测试中。

Import Dot

由于循环依赖关系,导入 . 的形式在测试中有用,不能成为测试包的一部分:

package foo_test

import (
    "bar/testutil" // also imports "foo"
    . "foo"
)

在这种情况下,测试文件不能在包 foo 中,因为它使用 bar/testutil,而 bar/testutil 导入了 foo。因此我们使用 import . 形式来让文件假装是包 foo 的一部分,即使它不是。除了这个例子,不要在你的程序中使用 import . 形式的导入。它使程序难于阅读,因为不清楚像 Quux 这样的名称是当前包中的一个顶级标识符还是导入的包中的。

In-Band Errors

在 C 语言和类似的语言中,函数通常会返回 -1 或 null 这样的值来发出错误或遗漏结果的信号:

// Lookup returns the value for key or "" if there is no mapping for key.
func Lookup(key string) string

// Failing to check a for an in-band error value can lead to bugs:
Parse(Lookup(key))  // returns "parse failure for value" instead of "no value for key"

Go 的支持多个返回值提供了更好地解决方案。而不是要求客户端检查 In-Band 错误值,函数应该返回额外的一个值来表明它其它的返回值是否有效。这个返回值可以是一个 error,或在没必要解释时返回一个布尔值。它应该是最后一个返回值。

// Lookup returns the value for key or ok=false if there is no mapping for key.
func Lookup(key string) (value string, ok bool)

这可以防止调用者错误的使用结果:

Parse(Lookup(key))  // compile-time error

并且鼓励更健壮和更可读的代码:

value, ok := Lookup(key)
if !ok  {
    return fmt.Errorf("no value for %q", key)
}
return Parse(value)

此规则适用于导出的函数,但对非导出的函数也是有用的。

当返回值如 nil,“”,0 和 - 1 这样在它们是函数的有效返回结果时,是可以接受的,即调用者不需要以不同于其它值的方式处理它们。

一些标准库函数,比如那些在包 strings 中的,返回 In-Band error 值。这极大的简化了字符串操作代码,代价是需要程序员进行更多的努力。通常来说,Go 代码应当为 error 返回额外的值。

Indent Error Flow

尽量将正常的代码路径保持在最小的缩进,并缩进错误处理,首先处理它。这通过允许快速扫描正常路径提高了代码可读性。例如,不要这样写:

if err != nil {
    // error handling
} else {
    // normal code
}

而要这样写:

if err != nil {
    // error handling
    return // or continue, etc.
}
// normal code

如果 if 语句有一个初始化语句,比如这样:

if x, err := f(); err != nil {
    // error handling
    return
} else {
    // use x
}

这可能需要将简短的变量声明移动到它们自己单独一行:

x, err := f()
if err != nil {
    // error handling
    return
}
// use x

Initialisms

名称中的首字母或首字母缩写词(例如 "URL" 或 "NATO")有一个一致的案例。例如,URL 应该表现为 URLurl(像在 urlPonyURLPony),永远不要用 Url。作为一个例子:写成 ServeHTTP 而不是 ServeHttp。用于具有多个初始化 “单词” 的标识符,使用例如 xmlHTTPRequestXMLHTTPRequest

这条规则也适用于 ID,当它是 identifier 的缩写时(几乎所有情况下都不是

这个规则也适用于 ID,当它是 identifier 的缩写时(几乎所有情况都不是 “ego”,“superego” 中的 “id”),所以写 appID 而不是 appId

通过 protocol buffer 编译器生成的代码不受此规则的约束。人类写的代码比机器写的代码有更高的标准。

Interfaces

Go 接口通常属于使用接口类型值的包,而不是实现这些值的包。实现包应该返回具体(通常是指针或结构体)类型:这样,可以将新方法添加到实现中,而无需进行大量重构。

不要在 API 的实现者端定义接口 “for mocking”;相反,设计 API 以便可以使用真实实现的公共 API 进行测试。

在使用接口前不要定义接口:没有实际使用的例子,甚至很难看出是否需要接口,更不用说它应该包含什么方法了。

package consumer  // consumer.go

type Thinger interface { Thing() bool }

func Foo(t Thinger) string { … }
package consumer // consumer_test.go

type fakeThinger struct{ … }
func (t fakeThinger) Thing() bool { … }
…
if Foo(fakeThinger{…}) == "x" { … }
// DO NOT DO IT!!!
package producer

type Thinger interface { Thing() bool }

type defaultThinger struct{ … }
func (t defaultThinger) Thing() bool { … }

func NewThinger() Thinger { return defaultThinger{ … } }

相反,返回一个具体类型,让使用者模拟生产者实现。

package producer

type Thinger struct{ … }
func (t Thinger) Thing() bool { … }

func NewThinger() Thinger { return Thinger{ … } }

Line Length

Go 代码中没有严格的行长度限制,但是要避免让人不舒服的长行。同样,当长行的可读性更好时,不要为了保持行较短来添加换行符 —— 例如,如果它们是重复的。

大多数时候,当人们用“不正常”的方式包装行时(在函数调用或函数声明的中间,可以说,或多或少有些例外),如果它们有合理数量的参数和合理简短的变量名时包装就没有必要了。长行似乎与长名称有关,摆脱长名称有很大帮助。

换句话说,可以因为要编写的内容的语义而换行(一般来说),而不是因为行的长度。如果你发现这会产生太长的行,那么更改名称或语义可能会得到比较好的结果。

实际上,这与关于函数应该有多长的建议完全相同。没有 “永远不会有超过 N 行的函数” 这样的规则,但是程序中肯定会存在行数太多,功能过于微弱的函数,而解决方案是改变这个函数边界的位置,而不是执着在行数上。

Mixed Caps

查看 https://golang.org/doc/effective_go.html#mixed-caps。即使它违反了其它语言中的约定,这也适用。例如未导出的常量是 maxLength 而不是 MaxLengthMAX_LENGTH

也可以查看 Initialisms

Named Result Parameters

想想在 godoc 中会是什么样子。指定的结果参数如下:

func (n *Node) Parent1() (node *Node)
func (n *Node) Parent2() (node *Node, err error)

在 godoc 中会造成口吃(stutter),最好这样使用:

func (n *Node) Parent1() *Node
func (n *Node) Parent2() (*Node, error)

另一方面,如果一个函数返回两个或三个相同类型的参数,或者,如果结果的含义从上下文看不清楚,那在某些上下文中添加名称可能会很有用。不要仅仅为了避免在函数内部用 var 声明变量就给结果参数命名;这是以不必要的 API 冗长为代价,换取了少量的实现简洁性。

func (f *Foo) Location() (float64, float64, error)

不如这样清晰:

// Location returns f's latitude and longitude.
// Negative values mean south and west, respectively.
func (f *Foo) Location() (lat, long float64, err error)

如果函数行数较少,那裸返回是可以的。一旦它是一个中等规模的函数,要明确返回值。推论:仅仅因为它使得能够使用裸返回就给结果参数命名是不值得的。文档的清晰性永远比在函数中节省一两行代码要重要。

最后,在某些情况下需要为结果参数命名,以便在延迟闭包中更改它。这总是可以的。

Naked Returns

查看 [Named Result Parameters](# Named Result Parameters)。

Package Comments

包的注释,就像 godoc 提供的所有注释一样,必须出现在 package 子句旁边,没有空行。

// Package math provides basic constants and mathematical functions.
package math
/*
Package template implements data-driven templates for generating textual
output such as HTML.
....
*/
package template

对于 main 包的注释,其它类型的注释可以放在二进制名称之后(并且如果它是第一个,就可以大写),例如,对于在目录 seedgen 中的 package main 你可以这样写:

// Binary seedgen ...
package main

或这样

// Command seedgen ...
package main

或这样

// Program seedgen ...
package main

或这样

// The seedgen command ...
package main

或这样

// The seedgen command ...
package main

或这样

// Seedgen ..
package main

这些是举例,它们的合理变体是可以接受的。

注意,以小写单词开头的句子不属于包注释的可接受选项,因为这些都是公开可见的,应该用正确的英语写,包括将句子的第一个单词大写。当二进制名称是第一个单词时,即使它与命令行调用的拼写不完全匹配,也需要将其大写。在 https://golang.org/doc/effective_go.html#commentary 查看有关注释约定的更多信息。

Package Names

包中名称的所有引用都将使用包名完成,因此你可以从标识符中省略该名称。例如,如果在包 chubby 中,不应该把类型名称定义为 ChubbyFile,否则使用者将写成 chubby.ChubbyFile。而是应该把类型命名为 File,使用者将写成 chubby.File。避免无意义的包名,如 util,common,misc,api,types,和 interfaces。

http://golang.org/doc/effective_go.html#package-nameshttp://blog.golang.org/package-names 查看更多。

Pass Values

不要仅仅为了节省几个字节而将指针作为函数参数传递。如果函数从始至终只是将它的参数 x 表示为 x,那么参数就不应该是指针。常见的实例包括传递一个字符串的指针 string,或一个接口值的指针,如 *io.Reader。在这两种情况下,值本身都是固定大小,可以直接传递。这个建议不适用于大型结构体,或即使很小但可能增长的结构体。

Receiver Names

一个方法的 receiver 的名称应该是它身份的反映;通常一个或两个字母的缩写就够了(比如把 "Client" 缩写为 "c" 或 "cl")。不要使用通用名,比如 "me","this" 或 "self",典型的面向对象语言标识符,赋予方法特殊的含义。在 Go 中,方法的 receiver 仅仅是另一个参数,因此应该相应的命名。名称不需要像方法参数那样具有描述性,因为它的作用是显而易见的,没有记录的目的。它可以非常短,因为它几乎出现在该类型的每个方法的每一行中,familiarity admits brevity。使用上也要一致:如果你在一个方法中把 receiver 称为 "c",不要在另一个方法中称它为 "cl"。

Receiver Type

在方法上选择使用值还是指针 receiver 可能比较困难,特别是对新手来说。如果有疑问,可以使用指针,但有时候,值 receiver 也是有意义的,通常是为了效率,例如对于小型不变的结构或基本类型的值。一些有用的指南:

Synchronous Functions

与异步函数相比,更喜欢同步函数(同步函数指直接返回其结果或在返回前完成任何回调或 channel 操作的函数)。

同步函数将 goroutines 本地化在一个调用中,更容易推断它们的寿命,以及避免泄漏和数据竞争。它们也更容易测试:调用者可以传递一个输入并检查输出,而不需要轮询或同步。

如果调用者需要,他们可以通过在单独的 goroutine 中调用此函数来轻松地提升并发性。但在调用端移除不需要的并发性非常困难,有时候是不可能的。

Useful Test Failures

测试失败时,应当提供有用的信息,来说明哪里出错了,输入什么,得到什么,期望什么。写一堆 assertFoo helpers 可能很诱人,但要确保 helpers 生成有用的错误消息。假设调试失败测试的人不是你,也不是你团队中的人。一个典型的 Go 测试失败如下:

if got != tt.want {
    t.Errorf("Foo(%q) = %d; want %d", tt.in, got, tt.want) // or Fatalf, if test can't test anything more past this point
}

注意这里的顺序是 actual != expected,消息也使用这个顺序。一些测试框架鼓励逆向编写这些内容:0 != x,"expected 0, got x", 等等。Go 不鼓励这样。

如果这看起来要大量的打字,你可以写一个表驱动测试

当使用具有不同输入的测试 helper 时,另一个消除失败测试歧义的通用技巧是用不同的 TestFoo 函数来封装每个调用者,因此测试失败具有以下名称:

func TestSingleValue(t *testing.T) { testHelper(t, []int{80}) }
func TestNoValues(t *testing.T)    { testHelper(t, []int{}) }

在任何情况下,您都有责任失败,并向将来进行代码调试的人提供有用的信息。

Variable Names

Go 中的变量名应该短而不是长。对于范围有限的局部变量尤其如此。首选 c 而不是 lineCount。首选 i 而不是 sliceIndex

基本原则:距使用名称的声明越远,该名称就必须越具有描述性。对于一个方法的 receiver 来说,一两个字母就足够了。常见的变量,如循环索引和 readers,可以用单个字母(i, r)。更不寻常的对象和全局变量需要更多的描述性名称。