[譯] Go 程式碼評審常見問題

空之古城發表於2018-07-11

本文翻譯自 Go 語言官方文件,收集了 Go 語言程式碼評審時的常見問題,適合掌握基本語法的初學者。

閱讀時間大約 15 分鐘

原文連結

Gofmt

程式碼提交前先跑一下 gofmt 工具,它能自動修復大多數形式化問題(對齊、換行等待)。

現在幾乎所有 Go 專案都在使用 gofmt,沒有使用的是因為它們在使用 goimports(它支援所有 gofmt 的功能,另外還可以規範化匯入行的寫法)。

下面我們討論的都是這兩個自動工具做不到的問題檢查。

Comment Sentences

用完整的句子註釋宣告,註釋往往以宣告的名稱開頭,以句點結束,像這樣:

// 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) { ...
複製程式碼

當然註釋的結尾用歎號或問號也沒問題。之所以這樣是使用 godoc 生成的文件可讀性很高。

更多資訊請參考effective go 文件

Contexts

context.Context 是 Go 語言中的一個標準型別,它內部往往包含跨越 API 和程式邊界的安全證書、跟蹤資訊、過期時間和取消訊號等資訊。

Go 程式的 RPC 呼叫時需要顯式地傳遞 context。

不要把 Context 放到結構型別中,把它放到引數列表裡,除非你的方法名改不了(比如方法的簽名必須與標準庫或第三方庫中的介面定義匹配)。如果放到引數列表裡,通常作為第一個引數,像這樣:

func F(ctx context.Context, /* other arguments */) {}
複製程式碼

你要是有資料需要傳遞,先考慮利用函式引數、接收器或全域性變數。如果資料真的適合放在 Context 中,要使用 Context.Value,不要自定義 Context 型別,也不要擴充套件 Context 介面。

Context 是不可變的,可以將相同的上下文傳遞給多個呼叫,它們共享資訊。

Copying

拷貝其他包中的結構要小心不預期的別名問題。舉個例子,bytes.Buffer 型別包含一個 []byte slice,Buffer 物件的拷貝可能跟原物件共同指向相同的記憶體,導致後續的呼叫產生不可預期的問題。看看下面這個例子:

package main

import (
"bytes"
"fmt"
)

func main() {
	a := bytes.Buffer{}
	a.Write([]byte("hello "))
	b := a
	a.Write([]byte("new world"))

	fmt.Printf("a: %s\n", a.Bytes())
	b.Write([]byte("old"))
	fmt.Printf("a: %s\n", a.Bytes())

	return
}
複製程式碼

執行的輸出是:

a: hello new world
a: hello old world
複製程式碼

總的來說,如果型別 T 有更改內部資料的行為,那就不要拷貝這個型別的值。

Declaring Empty Slices

定義一個空 slice,我們傾向於這麼寫:

var t []string
複製程式碼

而不是這樣:

t := []string{}
複製程式碼

前者方式定義了一個 nil 的 slice 值,後者定義了一個不空的長度為零的 slice。這兩種在很多情況下都一樣,比如它們的長度和容量都是零,但還是推薦前者。 在一些特殊的地方,這兩者不等價,比如 json 編碼中,前者編碼為 null,後者編碼為 json 陣列 []。

設計介面時,要避免這兩者引起的不同導致的細微的程式問題。

Crypto Rand

別用 math/rand 包生成(哪怕是一次性的)鍵值。沒有設定隨機種子時,生成器是完全可預測的,就算用 time.Nanoseconds() 設定隨機種子,熵也太少了。替代的方案是使用 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

所有頂級的、匯出的名字都需要有文件註釋,那些很重要的內部型別和函式也該如此。文件的規範參見這裡

Don't Panic

通常的錯誤處理要用 error 和多返回值,儘量不要用 Panic。

Error Strings

Error 的字串不要大寫開頭(特定名詞除外),也不要以標點結尾,因為它們經常被列印在錯誤輸出日誌中。 也就是說,要這樣定義錯誤:

fmt.Errorf("something bad") 
複製程式碼

而不是這樣:

fmt.Errorf("Something bad.")
複製程式碼

這樣使用方在日誌列印中會自然很多:

log.Printf("Reading %s: %v.", filename, err)
複製程式碼

Examples

增加一個新包時,要包含使用的例子:可以是可執行的例子,也可以包含完整呼叫的測試演示。

Goroutine Lifetimes

建立協程時,要清楚它是否會退出,以及什麼時候退出。

不再需要的協程,如果不妥善處理,會造成一些詭異的問題,比如:

  • 被收、發 channel 訊息阻塞住的協程會造成洩露;
  • 在結果都不需要的時候,修改輸入會導致無謂的併發資料競爭;
  • 傳送訊息給已經關閉的 channel 導致 Panic;

儘量保持併發程式碼簡單,協程的生命週期明確。要是真的做不到,就在文件中說明協程的退出時間和原因。

Handle Errors

不要用 “_” 方式丟棄錯誤,要檢查它來確保函式呼叫成功。處理錯誤,返回它們,甚至在必要的時候丟擲 Panic。更多詳情參見這裡

Imports

好的包名,在匯入時一般都不會發生衝突的。要儘量避免重新命名匯入包名,除非發生名字衝突。要真出現了衝突問題,優先考慮重新命名本地包或者特定工程的包的匯入。

匯入包要分組,分組之間用空行隔開,標準庫放到第一分組中:

package main

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

	"appengine/foo"
	"appengine/user"

    "github.com/foo/bar"
	"rsc.io/goversion/version"
)
複製程式碼

goimports 工具會幫助我們做這個事情。

Import Dot

點匯入的形式,可以方便有迴圈依賴的測試用例編寫,比如下面的程式碼:

package foo_test

import (
	"bar/testutil" // also imports "foo"
	. "foo"
)
複製程式碼

測試檔案 bar/testutil 匯入了 foo 包,如果測試用例需要匯入 bar/testutil, 那它就不能放在 foo 包下面,否則就會出現迴圈引用。這時候使用點匯入,就可以假裝測試用例是在 foo 包下面(其實是在 foo_test 包下面)。

除了上述情況,不要使用點匯入,它嚴重影響了程式碼的可讀性,你沒辦法區分一個 Quux 是當前包的一個識別符號還是某個匯入包的。

In-Band Errors

在 C 或者類似的語言中,一個常見的做法是通過異常返回值(比如 -1 或者 空指標)告知呼叫者發生了錯誤,我們管這種方式叫做內聯錯誤(In-Band Errors),像下面這樣:

// 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 語言中我們更推薦額外返回一個值來告知錯誤,比如 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)
This prevents the caller from using the result incorrectly:

Parse(Lookup(key))  // compile-time error
And encourages more robust and readable code:

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

這個規則對匯出和非匯出的函式都適用。

像 nil、""、0、-1 這樣的返回值,如果它們是合法的方法呼叫結果(判斷的標準是函式呼叫者可以用相同的邏輯處理這些值和其他值),那麼不增加 error 返回值也是合理的。

像 strings 這樣的標準庫,返回了內聯錯誤 值,這個簡化了字串處理程式碼,代價就是需要花費呼叫者更多的精力。

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”可以寫成“URL”或者“url”,在片語中可以是“urlPony”或者“URLPony”,但別寫成Url。另一個例子是要寫成“ServerHTTP”而不是“ServerHttp”。多個縮略詞在一起的名字,寫成“xmlHTTPRequest”或者“XMLHTTPRequest”都行。

再舉個“ID"的例子,表示“identifier”縮寫時,要寫成“appID”這樣,而不是“appId”。

protocol buffer 產生的自動化程式碼是個例外,寫程式碼對人和對機器的要求不能一樣。

Interfaces

總的來說,Go 的介面要包含在使用方的包裡,不應該包含在實現方的包裡。實現方只需要返回具體型別(通常是指標或結構),這樣可以方便地增加實現,而不需要擴充套件重構。

不要先定義介面再用它。脫離真實的使用場景,我們都不能確定一個介面是否有存在的價值,更別提設計介面的方法了。

測試時不要定義假介面給實現者用,反而是要定義公開的 API,用真實的實現進行測試。舉個例子,下面 consumer 是介面使用方,其實現和測試程式碼如下:

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{ … } }
複製程式碼

應該返回具體的型別,讓消費者來 mock 生產者的實現:

package producer

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

func NewThinger() Thinger { return Thinger{ … } }
複製程式碼

Line Length

在 Go 程式碼中沒有行長度的標準規定,避免不舒服的長度就好;類似的,長一些程式碼可讀性更強時,也不要刻意換行。

大多數非自然(在方法呼叫和宣告的過程中)的換行,都是可以避免的,只要選擇合理數量的引數列表和合適的變數名。一行程式碼過長,往往是因為程式碼中的各個名字太長了,去掉那些長名字就好了。

換句話說,在語義的分割點換行,而不是單單看行的長度。萬一你發現某一行太長了,要麼改名,要麼調整語義,往往就解決問題了。

這裡沒有一個“一個程式碼行最多不超過多少個字元”的規定,但是一定存在一行程式碼功能太雜,需要改變函式邊界的規則。

Mixed Caps

參考 mixed-caps,Go 語言推薦駝峰式命名。有一點點地方不同於其他語言:非匯出的常量要命名成 maxLength,而不是 MaxLength 或者 MAX_LENGTH。

Named Result Parameters & Naked Returns

比較下面兩個函式宣告,想象在文件中看到它們的感受:

func (n *Node) Parent1() (node *Node)
func (n *Node) Parent2() (node *Node, err error)
複製程式碼

這裡是第二個:

func (n *Node) Parent1() *Node
func (n *Node) Parent2() (*Node, error)
複製程式碼

有沒有感覺第一個顯得囉哩囉嗦?

我們再看另一種情況:一個函式返回兩三個相同型別的引數,或者返回引數的含義在上下文中不明確,給它們加上名字其實很有用。

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)
複製程式碼

不要僅僅以在函式中可以少宣告一個變數為由,給返回引數加上名字,偷懶是小,程式碼文件的可讀性是大。

一些隨手的簡單函式,不用命名返回引數也沒啥問題。一旦函式的規模大一些,就要顯式地指出它的返回值含義。不要為了給返回引數命名而命名,程式碼文件的清晰性才是第一位要考慮的。

最後,有一些模式中需要在 defer 閉包中修改返回值,這時候給返回值命名沒啥問題。

Package Comments

包註釋跟其他通過 godoc 展示的註釋一樣,必須臨近包的宣告,中間沒有空行,像這樣:

// 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 包的註釋,一般是按 main 所在的目錄來命名,比如一個檔案在 seedgen 目錄下,那相關注釋可以寫成下面任意一種:

// Binary seedgen ...
package main
or

// Command seedgen ...
package main
or

// Program seedgen ...
package main
or

// The seedgen command ...
package main
or

// The seedgen program ...
package main
or

// Seedgen ..
package main
複製程式碼

一個細節的問題是,用二進位制名做註釋的開頭時,首字母要不要大寫?答案是要!註釋是文件,所以要符合英文的文法,這就包括句子的首字母要大寫。至於文件中的大小寫跟實際的二進位制名字沒有嚴格匹配,那也只能這樣了。所以我推薦這樣的方式:

// The seedgen command ...
package main
or

// The seedgen program ...
package main
複製程式碼

Package Names

大部分對包內元素的引用,都是通過包名實現的,比如你有一個包叫 chubby,裡面的變數不必定義成 ChubbyFile(客戶使用的時候引用的方式是:chubby.ChubbyFile),而應該定義成 File(客戶使用的時候是 chubby.File)。

為減少引用衝突,避免用 util,common,misc,api,types 這樣的通用詞命包名。

Pass Values

別為了節省幾個位元組而傳遞指標引數,如果一個函式只用到了某個指標引數的 *x 形式,那就根本不應該傳指標引數。但這個建議不適用於大資料結構或可能會增長的資料結構的引數傳遞。

Receiver Names

方法接收器的名字,應該是一個簡寫,經常是型別字首的一兩個字母,不要用 me、this、self 這樣的通用名字。約定和一致性帶來簡潔性,如果你在一個方法中用 c 表示接收器,就別在其他方法中用 cl。

Receiver Type

Go 的初學者往往有一個事情選擇不好,就是方法的接收器是值還是用指標。一個基本的原則是把握不準的時候就用指標,但有些情況下用值接收更有道理,效能更好。這裡有一些有用的細則:

  • 方法需要改變接收器的內部值,那就必須用指標。
  • 接收器內含有 sync.Mutex 或者類似的同步域,那就必須指標,以避免拷貝。
  • 接收器是一個大資料結構或者陣列,指標會效率更高。
  • 如果接收者是一個結構、陣列、slice,且內部的元素有指標指向一些可變元素,那更傾向於用指標接收,來提供更明確的語義。
  • 如果是一個 map、func 或 chan,不要用指標接收;如果是一個 slice,並且方法中沒有改變其值(reslice 或 重新分配 slice),不要用指標。
  • 若接收者是一個小物件或陣列,概念上是一個值型別(比如 time.Time),並且沒有可變域和指標域,或者乾脆就是 int、string 這種基本型別,適合用值接收器。值接收器可以減少垃圾記憶體,通過它傳遞值時,會優先嚐試從棧上分配記憶體,而不是堆上。但保不齊在某些情況下編譯器沒那麼聰明,所以這個在用之前要測一下。
  • 最後,要是把握不準,就用指標。

Synchronous Functions

能用同步函式就不用非同步的,同步函式的含義是直接返回結果,或者在返回之前呼叫回撥函式或完成 channel 操作。

同步函式保持協程在呼叫時的本地性,更容易推斷協程的生命週期、記憶體洩漏和併發競爭,測試也更簡單。

如果呼叫者需要併發處理,它可以很簡單地開一個單獨的協程;但對一個非同步函式來說,呼叫者想改成同步的就太難了,有時候就根本不可能。

Useful Test Failures

測試的失敗分支,需要加上有用的診斷資訊,包括輸入、實際輸出、期望輸出都是什麼,像這樣:

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
}
複製程式碼

寫的時候注意實際輸出和期望輸出別寫反了。

如果你覺得程式碼量太大,可以嘗試表驅動測試方案。

另外一個區分測試失敗分支的方法是使用不同的測試函式名,像這樣:

func TestSingleValue(t *testing.T) { testHelper(t, []int{80}) }
func TestNoValues(t *testing.T)    { testHelper(t, []int{}) }
複製程式碼

不管哪種方法,目標是給以後維護你程式碼的人,提供足夠多的診斷資訊。

Variable Names

Go 語言變數名短比長好,尤其是那些作用域很小的本地變數。用 c,不要用 lineCount;用 i,不要用 sliceIndex。

基本準則是:宣告到使用的距離越遠,變數名字就越詳盡。方法接受者的名字,一兩個字元就夠了,通常的迴圈下標、讀取器引用,一個字元(i,r)足夠;那些不常見的,全域性的變數,名字要起的說明性更強一些。

相關文章