[譯] Go 代碼評審常見問題

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

閱讀時間大約 15 分鐘node

原文連接git

Gofmt

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

如今幾乎全部 Go 項目都在使用 gofmt,沒有使用的是由於它們在使用 goimports(它支持全部 gofmt 的功能,另外還能夠規範化導入行的寫法)。golang

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

Comment Sentences

用完整的句子註釋聲明,註釋每每以聲明的名稱開頭,以句點結束,像這樣:api

// 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)足夠;那些不常見的,全局的變量,名字要起的說明性更強一些。

相關文章
相關標籤/搜索