👀

[和訳]Go Code Review Comments

2024/08/27に公開

この記事は、Go Code Review Commentsの翻訳記事です。
原典の情報に加えて、部分的に補足説明を追加しています。
文の順序や構造は自然な日本語になるように調整していますが、段落は原典と対応するようにしています。

日本語にすることが適切でない単語や表現については、このようにインラインコードで示しています(通常のコード片も同様に表示)。

著作権表示

翻訳元であるGo wikiの内容は基本的にCC BY 4.0で、コード部分は修正BSDライセンスで提供されています
本記事は、基となる記事Go Code Review Commentsに、CC BY 4.0、修正BSDライセンスで許可されている改変を加えたものです。

以下は、原文Go Code Review Commentsの著作権表示です。
(修正BSDのライセンス表示をもって、CC BY 4.0の著作者表示も兼ねています)

Copyright (c) 2009 The Go Authors. All rights reserved.

Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions are
met:

   * Redistributions of source code must retain the above copyright
notice, this list of conditions and the following disclaimer.
   * Redistributions in binary form must reproduce the above
copyright notice, this list of conditions and the following disclaimer
in the documentation and/or other materials provided with the
distribution.
   * Neither the name of Google Inc. nor the names of its
contributors may be used to endorse or promote products derived from
this software without specific prior written permission.

THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

Go Wiki: Go Code Review Comments

この記事は、Go言語でよくあるコードレビューコメントをまとめたものです。
実際にレビューで指摘する際に、各項目のリンクを提示することで詳細な説明を参照できるようにしています。
典型的な問題に関するリストであり、網羅的なスタイルガイドではないことに注意してください。

Effective Goの補足として利用できます。

テストに関するコメントは、Go Test Commentsで確認できます。

コードスタイルに関しては、Googleが公開しているGo Style Guideも参考になります。

軽微なものであっても、このページを編集する前に必ず議論してください。
多くの人が意見を持っています。
このページは編集戦争の場所ではありません。

Gofmt

gofmtを実行してください。
これにより、スタイルに関する問題のほとんどが自動的に修正されます。
世にあるGoコードのほとんどはgofmtを使用しています。
このページでは以降、機械的に修正できないスタイルに絞って説明します。

gofmtの代替として、goimportsがあります。
goimportsgofmtの上位互換のツールであり、必要に応じてインポート行を追加(または削除)します。

Comment Sentences

https://go.dev/doc/effective_go#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のプログラムでは、入力RPC・HTTPリクエストから外部へのリクエスト発行まで、関数呼び出しチェイン全体でContextを明示的に渡します。

大抵の関数では、Contextを第1引数として受け取るべきです。

func F(ctx context.Context, /* その他の引数 */) {}

関数の処理が絶対にrequest-specificでない場合はcontext.Background()を使用できますが、必要がないと思ってもContextを渡す方が無難です。
デフォルトではContextを渡すようにしておき、context.Background()を直接使用するのは、代替手段が誤りであるという理由がある場合に限ります。

構造体のメンバーにContextを追加しないでください。
代わりに、それを渡す必要がある各メソッドにctx引数を追加してください。
例外は、標準・サードパーティライブラリのインタフェースにシグネチャを合わせる必要があるメソッドです。

独自のContext型を作成したり、Context以外のインタフェースを関数シグネチャで使用しないでください。

アプリケーションデータを引き回す場合、基本的には引数、レシーバ、グローバル変数に格納することを検討し、本当にContextに属する場合にのみContextに格納してください。

Contextはイミュータブル(不変)なので、同じデッドライン、キャンセルシグナル、クレデンシャル、親トレースなどを共有する複数の呼び出しに同じctx変数を渡すことは問題ありません。

Copying

予期しないaliasingを避けるために、他のパッケージから構造体をコピーする際には注意してください。
たとえば、bytes.Buffer型には[]byteスライスが含まれています。
このとき単にBufferをコピーすると、コピーしたスライスは元のスライスと同じ配列を参照する可能性があります。
これにより、後続のメソッド呼び出しに予期しない副作用が生じるかもしれません。

一般的に、ポインタ型*Tに関連付けられたメソッドを持つ型Tの値をコピーしないでください。

Crypto Rand

使い捨てであっても、math/randパッケージを使用して鍵を生成しないでください。
シードが設定されていない場合、生成器は完全に予測可能です。
time.Nanoseconds()でシードを設定した場合でも、エントロピーはわずか数ビットしかありません。
代わりにcrypto/randReaderを使用し、テキストが必要な場合は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)
}

Declaring Empty Slices

空のスライスを宣言する場合、以下のように宣言することを推奨します。

var t []string

以下のように宣言するのは避けてください。

t := []string{}

前者は値がnilのスライスを宣言し、後者はnilではないが長さがゼロのスライスを宣言します。
機能的には等価(len()cap()がどちらもゼロ)ですが、値がnilのスライスの方が好ましいスタイルです。

特定の状況では、長さゼロのスライス([]string{})が適切な場合もあります。
たとえば、JSONオブジェクトをする場合、nilスライスはnullにエンコードされますが、[]string{}はJSON配列[]にエンコードされます。

インタフェースを設計する際、nilスライスとnilではない長さゼロのスライスの区別しないようにしてください。

Goのnilに関する詳細な議論については、Francesc CampoyのUnderstanding Nilを参照してください。

Doc Comments

トップレベルのエクスポートされる名前には、すべてにドキュメントコメントを付けるべきです。
また、エクスポートされていない型や関数宣言でも、重要なものにはドキュメントコメントを付けるべきです。
コメントの慣習についての詳細はhttps://go.dev/doc/effective_go#commentaryを参照してください。

Don’t Panic

https://go.dev/doc/effective_go#errorsも参照してください。
通常のエラーハンドリングにpanicを使用しないでください。
errorを含む複数の戻り値を使用してください。

Error Strings

後続のコンテキストに続いて表示されることが多いため、エラー文字列は大文字で始めたり、句読点で終わらせたりすべきではありません(固有名詞や略語で始まる場合を除く)。
つまり、fmt.Errorf("something bad")を使用し、fmt.Errorf("Something bad")を使用しないでください。
そうすると、log.Printf("Reading %s: %v", filename, err)がメッセージ中に余分な大文字が含まれないようにフォーマットされます。
ログ出力は暗黙に行志向であり、他のメッセージ内に組み込まれることはないため、このルールは適用されません。

Examples

新しいパッケージを追加する際には、実行可能なExampleや完全な呼び出し手順を示す単純なテストを書くことで、意図した使用方法の例を含めてください。

こちらも参考にしてください: testable Example() functions.

Goroutine Lifetimes

groutineを生成する際には、それがいつ終了するのか、またそもそも終了するのかを明確にしてください。

goroutineは、チャネルの送受信でブロックされることによってリークする可能性があります。
これは、GCがブロックされたチャネルが到達不能であってもgoroutineを終了させないためです。

不要になったgroutineをそのままにしておくと、goroutineがリークしなかったとしても次のような微妙で原因究明が難しい問題を引き起こす可能性があります。
Closeされたチャネルに対するSendpanicを引き起こします。
結果が不要になった後でも、使用中の入力を変更することでデータ競合を引き起こすことがあります。
そして、goroutineが任意の時間実行したままになっていると、メモリ使用量が予測不能になる可能性があります。

goroutineのライフタイムを明確にするため、並行コードをできるだけシンプルに保つようにしてください。
それができない場合は、goroutineがいつ、なぜ終了するのかをドキュメント化してください。

Handle Errors

https://go.dev/doc/effective_go#errorsも参照してください。
_を使用してエラーを破棄しないでください。
関数がエラーを返す場合は、関数が成功したことを確認するためにエラーをチェックしてください。
エラーに対処するか、エラーを返すか、本当に例外的な状況であればpanicしてください。

Imports

名前の衝突を避ける目的以外で、インポート名を変更しないでください。
適切なパッケージ名であれば、インポート名を変更する必要はないはずです。
衝突時には、よりローカルでプロジェクト固有な方のインポート名の変更を推奨します。

インポートはグループごとに整理され、間に空行が入ります。
標準ライブラリのパッケージは常に最初のグループに配置します。

package main

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

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

これはgoimportsが自動で行ってくれます。

Import Blank

副作用のためだけにインポートされるパッケージ(import _ "pkg"を使用)は、プログラムのmainパッケージか、それを必要とするテストでのみインポートすべきです。

Import Dot

import .形式は、循環依存によってテスト対象パッケージの一部にできないテストで便利です。

package foo_test

import (
    "bar/testutil" // このパッケージもfooをインポートしている
    . "foo"
)

この例では、fooをインポートしているbar/testutilをインポートしているため、テストファイルをfooパッケージに配置できません。
ここで、import .形式を使用するとテストファイルがfooパッケージの一部であるかのように振る舞わせることができます。
この1つのケースを除いて、import .を使用しないでください。
import .によってQuuxのような名前がそのパッケージのトップレベル識別子なのか、インポートされたパッケージの識別子なのかが不明確になり、可読性が低下します。

In-Band Errors

C言語などの言語では、エラーや結果が見つからないことを示すために、-1nullのような値を返すことが一般的です。

// Lookup はKeyに対応する値を返し、見つからない場合は""を返す。
func Lookup(key string) string

// in-bandなエラーをチェックしないことで以下のようなバグにつながる。
Parse(Lookup(key))  // "keyに対応する値がありません"を返すべきところ、"パースに失敗しました"というエラーになる

Goは複数戻り値をサポートしているので、これが良い解決になります。
呼び出し側にin-bandなエラーをチェックさせる代わりに、関数は他の戻り値が有効かどうかを示すための追加の値を返すべきです。
この返り値はerrorか、説明が不要な場合はboolであるべきです。
また、errorboolは最後の戻り値であるべきです。

// Lookup はKeyに対応する値を返し、見つからない場合はok=falseを返す。
func Lookup(key string) (value string, ok bool)

これで、呼び出し元が結果を不正に使用することを防ぐことができます。

Parse(Lookup(key))  // コンパイル時エラー

そして、より堅牢で読みやすいコードになります。

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

このルールはエクスポートされた関数に適用されますが、そうでない関数にも有用です。

nil""0-1などの返り値は、関数の有効な結果である場合には問題ありません。
つまり、呼び出した場所で他の値と異なる処理を行う必要がない場合です。

stringsパッケージのような標準ライブラリの一部の関数は、in-bandなエラー値を返します。
これは、プログラマにより多くの注意を要求する代わりに、文字列操作コードを大幅に簡素化します。
一般的には、Goのコードはエラーのために追加の値を返すべきです。

Indent Error Flow

通常のコードパスを最小限のインデントに保ち、エラーハンドリングを先に実行して、そこにインデントを付けるようにしましょう。
これによって、正常系のコードパスを素早く流し読みできるため、コードの可読性が向上します。

if err != nil {
    // エラー処理
} else {
    // 正常系のコード
}

上の例は、以下のように書き換えてください。

if err != nil {
    // エラー処理
    return // または`continue`など
}
// 正常系のコード

以下のように、if文に初期化ステートメントがある場合は、

if x, err := f(); err != nil {
    // エラー処理
    return
} else {
    // use x
}

変数宣言を独立した行に移動する必要があるかもしれません。

x, err := f()
if err != nil {
    // エラー処理
    return
}
// use x

Initialisms

頭文字語や略語(例:URLNATO)を含む単語は一貫したケースを持つべきです。
たとえば、URLURLurlとして表示されるべきで(urlPonyURLPony)、Urlにはしないでください。
例として、serve httpServeHttpではなくServeHTTPとします。
複数の単語を持つ識別子の場合は、xmlHTTPRequestXMLHTTPRequestのようにします。

このルールは、IDidentifierの略語である場合にも適用されます(idegosuperegoの並びで使われる場合を除いてほとんど全ての場合で)。
そのため、appIdではなくappIDとしてください。

protocol bufferコンパイラによって生成されたコードは、このルールの対象外です。
人間が書いたコードには、機械が生成したコードよりも高い水準が求められます。

Interfaces

Goのinterfaceは通常、そのinterface型を満たす値を実装するパッケージではなく、使うパッケージに属します。
interfaceを実装するパッケージは具象型(通常はポインタや構造体)を返すべきです。
これにより、実装に新しいメソッドを追加する際に大規模なリファクタリングを必要とせずに済みます。

「モックのために」APIの実装側でinterfaceを定義しないでください。
代わりに、実際の実装の公開APIを使用してテストできるようにAPIを設計してください。

使用する前にinterfaceを定義しないでください。
実際の使用例がないと、interfaceが必要かどうかさえわかりにくく、どのメソッドを含めるべきかもわかりません。

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" { … }
// このように実装してはいけません!!!
package producer

type Thinger interface { Thing() bool }

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

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

代わりに具象型を返し、consumerパッケージがproducerの実装をモックするようにしてください。

package producer

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

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

Line Length

Goコードには厳格な行長制限はありませんが、不快なほど長すぎる行は避けてください。
同様に、読みやすい場合には長いままにしておき、改行入れるべきではありません。
たとえば、同じ処理が繰り返される場合などです。

「不自然な」改行が多い場合(例外はありますが、関数呼び出しや関数宣言の途中など)、適切な数の引数と適切に短い変数名があればそれらは不要なはずです。
長い行は長い名前とセットになることが多いため、名前を短くすることでかなり改善されます。

言い換えると、(一般的なルールとして)書いている内容のセマンティクス(意味的な内容)に基づいて改行するようにし、長さそのものを理由に改行しないでください。
行が長すぎると感じた場合は、名前やセマンティクスを変更してみることで、良い結果が得られるでしょう。

実は、関数がどれくらい長くなるべきかについても同じことが言えます。
「N行を超える関数を書いてはいけない」というルールはありませんが、確かに長すぎる関数や、小さすぎて繰り返しの多い関数があると問題になります。
そして、その解決策は関数の境界を変更することであり、行数を数えることではありません。

Mixed Caps

https://go.dev/doc/effective_go#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では繰り返しになります。
以下のようにすると良いでしょう。

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

一方で、関数が同じ型の値を2つまたは3つ返す場合や、結果の意味がコンテキストから明確でない場合は、名前付き戻り値が有用な場合もあります。
しかし、ただ関数内で変数を宣言するのを避けるためだけに使うべきではありません。
(これは、実装を少しだけ簡潔にする代わりに、APIが不必要に冗長になります。)

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

上記は以下の例よりもわかりにくいです。

// Location はfの緯度と経度を返します。
// 負の値はそれぞれ南と西を意味します。
func (f *Foo) Location() (lat, long float64, err error)

関数が数行程度の場合は、naked returnで問題ありませんが、中規模以上の関数になったら、戻り値の名前を明示的にするべきです。
逆に、naked returnを使うためだけに名前付き戻り値を使う価値はありません。
ドキュメントの明確さは、関数内の1、2行を節約するよりも重要です。

最後に、deferして使うクロージャ内で変更するために名前付き戻り値が必要な場合もあります。
これは常に問題ありません。

Naked Returns

引数なしのreturn文は、自動的に名前付き戻り値を返します。
これはnaked returnとして知られています。

func split(sum int) (x, y int) {
    x = sum * 4 / 9
    y = sum - x
    return
}

Named Result Parametersも参照してください。

Package Comments

godocで表示されるすべてのコメントと同様に、パッケージコメントはpackage節に空行を挟まず隣接する必要があります。

// math パッケージは基本的な定数と数学的関数を提供します。
package math
/*
template パッケージは、HTMLなどのテキスト出力を生成するためのデータ駆動のテンプレートを実装します。
....
*/
package template

package mainのコメントについては、バイナリ名の後に他のスタイルのコメントを使用しても問題ありません(最初に来る場合は大文字にできます)。
たとえば、seedgenディレクトリ内のpackage mainの場合、以下のように書くことができます。

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

これらの例や、これらの合理的なバリエーションは受け入れられます。

パッケージコメントでは、文の最初の単語を小文字で始めることは許可されていません。
これらは公開されるため、文の最初が大文字である正しい英語で書かれるべきです。
バイナリ名が最初の単語の場合、コマンドラインで呼び出すときのスペルと厳密に一致しない場合でも、大文字で始める必要があります。

コメントの慣習についての詳細はhttps://go.dev/doc/effective_go#commentaryを参照してください。

Package Names

あるパッケージ内の名前への参照は、必ずそのパッケージ名を付けて行われるため、識別子からその名前を省略できます。
たとえば、chubbyパッケージ内にいる場合、chubby.ChubbyFileと呼び出されるためChubbyFileという型名は冗長です。
代わりに、chubby.Fileと呼び出せるようにFileという名前を付けてください。
utilcommonmiscapitypesinterfacesのような意味のないパッケージ名は避けてください。
詳細はhttps://go.dev/doc/effective_go#package-nameshttps://go.dev/blog/package-namesを参照してください。

Pass Values

数バイトを節約するためだけに、関数の引数としてポインタを渡さないでください。
関数が引数x*xとしてのみ参照する場合、ポインタ引数にすべきではありません。
これには、文字列(*string)やインタフェース値(*io.Reader)へのポインタを渡す場合も含まれます。
どちらの場合も、値自体は固定長であり、直接渡すことができます。
このアドバイスは、大きな構造体やサイズが大きくなりうる構造体には適用されません。

Receiver Names

メソッドのレシーバの名前は、そのレシーバのアイデンティティを反映するべきです。
多くの場合1, 2文字の略語で十分です(Clientの場合はcclなど)。
メソッドに特別な意味を持たせるオブジェクト指向言語のような、methisselfなどの名前は使用しないでください。
Goでは、メソッドのレシーバは単なる引数とは別のパラメータであるため、それに応じた名前を付けるべきです。
役割が明確で、ドキュメントとしての目的を果たさないため、メソッドの引数ほど説明的である必要はありません。
その型のすべてのメソッドのほぼすべての行に表示されるため、非常に短くても構いません。
親しみやすさによって簡潔さは許容されます。
また、一貫性を保つことも重要です。
あるメソッドでレシーバをcとする場合、別のメソッドでclとはしないでください。

Receiver Type

特に新人Goプログラマにとって、メソッドで値またはポインタレシーバのどちらを使用するか選択することは難しいかもしれません。
迷った場合はポインタレシーバを使うべきですが、効率のために値レシーバを使用する方が適切なこともあります。
たとえば、小さな変更されない構造体や基本型の値などです。
以下にいくつかの有用なガイドラインを示します。

  • レシーバがmapfuncchanの場合や、レシーバがsliceでメソッドがそれを再スライスしたり再割り当てしない場合、ポインタレシーバにしないでください。
  • メソッドがレシーバを変更する必要がある場合、ポインタレシーバでなければなりません。
  • レシーバがsync.Mutexなどの同期フィールドを含む構造体の場合、コピーを避けるためにポインタレシーバでなければなりません。
  • レシーバが大きな構造体や配列の場合、ポインタレシーバの方が効率的です。どの程度を大きいとするかは、その要素をすべてメソッドに引数として渡すことを考えてください。それが大きすぎると感じる場合、レシーバとしても大きすぎます。
  • 他の関数やメソッドが、同時に実行されていたり、メソッドから呼び出されたりして、レシーバを変更する可能性がありますか? 値型はメソッドが呼び出されるときにレシーバのコピーを作成するため、外部の更新はそのレシーバには適用されません。元のレシーバに変更を反映する必要がある場合、ポインタレシーバでなければなりません。
  • レシーバが構造体、配列、スライスで、その要素のいずれかが変更される可能性のあるものへのポインタである場合、読み手に意図が明確になるようにポインタレシーバを選択してください。
  • レシーバが小さな配列や構造体で、元々値型(たとえばtime.Time型)の場合や、変更可能なフィールドやポインタを持たない場合、あるいはintstringなどの単純な基本型である場合、値レシーバが適しています。値がメソッドに渡される場合、ヒープではなくスタック上にコピーされ、生成されるガベージが減ることがあるからです(コンパイラはこのヒープ割り当てを避けようとしますが、必ずしも成功するわけではありません)。ただし、この理由だけで値レシーバを選ぶ前に、必ずプロファイリングを行ってください。
  • レシーバの種類を混在させないでください。すべてのメソッドのレシーバをポインタまたは値のどちらかに統一してください。
  • 最後に、迷った場合はポインタレシーバを使用してください。

Synchronous Functions

非同期関数よりも同期関数(結果を直接返すか、コールバックやチャネル操作を完了してから終了する関数)を選択してください。

同期関数はgoroutineを呼び出しの間に局在させるため、そのライフタイムを理解しやすくし、リークやデータ競合を回避しやすくなります。
また、テストもしやすくなります。
呼び出し元は入力を渡し、出力を確認するだけで済み、ポーリングや同期が必要ありません。

呼び出し側がより多くの並行性を必要とする場合、別のgoroutineから関数を呼び出すことで簡単に追加できます。
反対に、呼び出し側で不要な並行性を削除することは非常に難しいか、不可能です。

Useful Test Failures

テストは、何が間違っていたのか、どのような入力があったのか、実際に得られた結果は何か、期待された結果は何か、を示す有用なメッセージとともに失敗するべきです。
assertFooのようなヘルパをたくさん書くのは魅力的かもしれませんが、そのヘルパが有用なエラーメッセージを生成することを確認してください。
失敗したテストをデバッグする人があなたでなく、あなたのチームでもないと仮定してください。
典型的なGoテストは以下のように失敗します。

if got != tt.want {
    t.Errorf("Foo(%q) = %d; want %d", tt.in, got, tt.want) // これ以降何もテストできない場合は、Fatalfを使用
}

ここでの順序はactual != expectedであり、メッセージもその順序になっています。
一部のテストフレームワークは、逆に書くことを奨励しています。
0 != x“expected 0, got x”などですが、Goではそうではありません。

もしそれが多くの入力を必要とするようであれば、table-driven testを書くことを検討してください。

テスト失敗時の曖昧さをなくすためのテクニックとして、それぞれの呼び出しを異なるTestFoo関数でラップする方法もあります。
これにより、テストがその名前で失敗するようになります。

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

いずれにせよ、将来あなたのコードデバッグする人に対して、有用なメッセージとともに失敗する責任はあなたにあります。

Variable Names

Goの変数名は長いよりも短い方が良いです。
特にスコープが限られたローカル変数について顕著であり、lineCountよりもcsliceIndexよりもiを選ぶべきです。

使われる場所が宣言から遠いほど説明的な名前が必要になるという基本的なルールがあります。
メソッドのレシーバの場合、1、2文字で十分です。
ループのインデックスやreaderなどの一般的な変数は1文字で構いません(ir)。
より珍しいものやグローバル変数には、より説明的な名前が必要です。

より詳細については、Google Go Style Guideを参照してください。

GitHubで編集を提案

Discussion