Zenn
Closed25

Go Review Commentsを読む

Go
OsamuOsamu

目的

  • 業務でGoのレビューをすることが多くなってきたので改めて見直したい
  • アウトプットを兼ねて、読んだログを残したい
    • 気になる部分があればスクラップに書いていきます
  • 最後まで読んだらゴール
OsamuOsamu

Gofmt

フォーマットはちゃんと行おう。
gofmtを使用すると自動的にフォーマットしてくれるので活用をする。

gofmt <フォーマットしたいディレクトリやパス>
OsamuOsamu

Contexts

context.Context型はAPIやプロセスを超えて、認証情報、トレース情報、期限、キャンセルシグナルを伝える。
Contextは関数呼び出しの連鎖全体で明示的に渡され、Contextを使用するほとんどの関数は、最初のパラメータとしてContextを受け入れる必要がある。

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

また構造体のフィールドとしてContextを持たせてはいけなく、必要であれあればメソッドの引数として渡す。
独自のcontext.Context型を使用したり、context.Context以外のinterfaceを関数の引数に使ってはならない。
contextはimmutabule（変更不可能）なので、認証情報やキャンセルシグナルなどを共有する複数の呼び出しに対して、ctxを渡しても大丈夫。

OsamuOsamu

Copying

構造体がポインタを持っている時は気をつけよう。
コピーした構造体の内部のポインタがコピーされるので、値が共有され低としない挙動が出る時がある。

ex)

package main

import "fmt"

type Buffer struct {
    data []byte
}

func (b *Buffer) Add(s string) {
    b.data = append(b.data, []byte(s)...)
}

func main() {
    buf1 := Buffer{data: []byte("hello")}
    buf2 := buf1 // buf1をコピー

    buf1.Add(", world") // buf1にデータを追加
    fmt.Println("buf1:", string(buf1.data)) // buf1: hello, world
    fmt.Println("buf2:", string(buf2.data)) // buf2: hello, world  <- buf2も変わってしまう！
}
OsamuOsamu

Crypto Rand

math/randパッケージを使用してキーを作成してはならない。（シードされていないジェネレーターは完全に予測可能）
time.Nanoseconds()を利用したシード値の作成も可能ではあるが、短時間に大量のキーを作成するとパターンが予測可能になる可能性もあるので注意。

代わりにcrypto/randのReaderを使用するべき。
テキストが必要であれば16進数か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)
}
OsamuOsamu

Declaring Empty Slices

スライスはnilスライスで宣言した方が好ましい。

// こっち（空スライス）よりも
t := []string{}

// こっち（nilスライス）の方が良い
var t []string

JSONにエンコードする際も以下の違いがある

  • nilスライス → null
  • 空スライス → []

JSON側の仕様で空の配列を利用する時は空スライスでも良いが、特段理由がなければnilスライスで宣言をして、明確な意図を用いて区別していく。

OsamuOsamu

Don't Panic

通常のエラー処理にはpanicを使用しない。
エラーと複数の戻り値を使用する。

OsamuOsamu

Error Strings

エラーメッセージは大文字で始めず、句読点（. , ! ?など）で終わらないようにする。
エラーメッセージをフォーマットする時、そのような状況だと不自然な印象になるから。

OsamuOsamu

Examples

新しいパッケージを追加する際には実行可能なExampleやコールシーケンスを示す簡単なテストなど、意図した使用例を含めるようにする。

OsamuOsamu

Goroutine Lifetimes

goroutineを作成する時は、いつ終了するかを明確にする。
goroutineはチャネルの送信や受信でブロックされると終了できなくなることがあり、goroutineが動き続てしまう可能性がある（goroutineリーク）
また、データ競合、メモリを使用し続けるパフォーマンス低下、panicの原因にもつながる。

OsamuOsamu

Handle Errors

エラーは_を使用して破棄してはいけない。
errorを返す関数であるのであれば、実行直後に必ず成功したかどうかを確認する。

OsamuOsamu

Imports

import分は名前の衝突を避けるべきであり、別名でインポートしてしまわないようにする。
もし名前の衝突が起きた場合は、プロジェクト固有のパッケージに別名をつける

また、import分は以下のグループごとに改行を挟んで整理される。

  1. 標準パッケージ
  2. Publicパッケージ
  3. 自パッケージ
package main

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

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

goimportsを使用するとよしなにやってくれる

OsamuOsamu

Import Dot

ドットインポートは循環依存など、特定の状況下以外は使用しない。
非常にプログラムが読みにくくなるので。

package foo_test

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

Indent Error Flow

正常な処理を最小限のインデントに保つようにする。
エラー処理を先に行い、elseなどは極力使用しないようにし、ネストを深くしない。

こっちの処理より

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

こっちの処理にする

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

Initialisms

初期化語や頭文字をとった単語の場合、大文字か小文字かは一貫している必要がある。
例えば以下のような言葉

  • URL
  • HTTP
  • NATO
  • ID

もし単語の先頭に来る場合であれば必ず統一をする。
xmlHttpRequestとう言うように書くのでは無く、以下のように書く。

  • xmlHTTPRequest
  • XMLHTTPRequest

統一することで可読性が向上し、検索しやすくなる。

自動生成やProtocolBufferが作成したコードに関しては適用外なので注意。
機械的に正絵師されたコードよりも、人間が書いたコードの方が品質が求められる。

OsamuOsamu

Line Length

Goに行長制限はないが、あまりにも長すぎる行は避けるべき。
繰り返しが多い場合は無理に改行しなくてもok
意味に基づいて正しく改行する。

OsamuOsamu

Mixed Caps

エクスポートされない識別子（変数、定数、関数）の名前はMixed Caps（混合キャメルケース）を使用する。
MaxLengthやMAX_LENGTHでは無く、maxLengthと書く。

OsamuOsamu

Named Result Parameters

godocでの見やすさを考慮すると、戻り値に名前をつけることが適切ではない場合がある。

以下よりも

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)
OsamuOsamu

Package Names

パッケージ名と被るパッケージ内の識別子に関しては省略する。
chubbyパッケージの場合、chubby.ChubbyFileとするのでは無く、chubby.Fileとすると冗長にならずに済む。

OsamuOsamu

Pass Values

小さい値を関数に渡す時、ポインタでは無く直接値を渡す。
小さい値であればコピーコストが小さくなるので、ポインタの値参照をするより処理が早くなる。

例外

大きい構造体や将来的に大きくなる可能性がある小さい構造体は、コピーにコストがかかるので、ポインタにて値を渡した方がいい。

OsamuOsamu

Receiver Names

メソッドのレシーバ名は通常1~2文字で十分。（Clientであればcやcl）
me、this、selfのようなオブジェクト指向にあるような名前は使用しない。
レシーバはメソッドのほぼ全ての行に出現するので、短くすることで可読性が上がる。

OsamuOsamu

Synchronous Functions

関数は非同期関数よりも同期関数を優先的に使用する。
同期関数であれば以下のことがメリットとしてある。

  • goroutineが局所的になり、競合やリークのリスクが少なくなる
  • 挙動が想定しやすいのでテストが容易になる
  • 並行処理が必要なタイミングで適切にgoroutineを追加できる
OsamuOsamu

Useful Test Failures

テストは何が間違っていたのか、どのような入力で何が得られたのか、実際に得られたものが何で、期待値が何か、など有用なメッセージとともに失敗をするべき。
失敗したテストをデバックする人はあなたでもあなたのチームでもないと考える。

ex)

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の順番でチェックしていく。

また、複数の入力に対してテストヘルパーを使用する場合は、呼び出し元のテストでラップをしてあげると、失敗した時のケースが明確になる。

OsamuOsamu

Variable Names

Goの変数名は長くするより短くするべき。
スコープが短いほど特にそう。

基本的なルールとしてはスコープが短いほど変数名は短く、スコープが長いほど変数名時は説明的な名前にする。

OsamuOsamu

最後に

目的としては読めたのでdoneです。
ドキュメント自体は、なんとなく日頃Goを書いていて意識していたことが明文化されていたのでとてもよく、今後Pull Requestのレビューを行う際にも役に立ちそうです。

和訳している有志の方も他にいらっしゃったのでそちらを参照時に使うのもいいかもしれません。

このスクラップは2ヶ月前にクローズされました