この記事では2人目の鵜飼文敏氏の基調講演についてレポートを書きたいと思います。この基調講演では
Go言語の可読性レビュー
Go Readability Approver
まずはじめに、
「Go Readability Approver」
可読性のあるGo言語のコード
同氏は、
- 明瞭・
簡潔 - 使いやすいAPI
- 適切なコメント
- 素直なコードフロー
特にゴルーチンを使った並行プログラミングは、
Go言語で読みやすいコードを書くための優れたツールとして、
- go fmt：コードをGo言語の標準フォーマットに変換するツール
- go vet：間違えやすいコードを指摘するツール
- golint：スタイルの問題を指摘するツール
- godoc：コードからAPIドキュメントを作るツール
しかし同氏は、
この講演では、
- ミス／
バグはないか - 見やすくレイアウトされているか
- コードフローはわかりやすいか
- APIはわかりやすいか
エラー
Must-を使った初期化
Go言語では、_で受け取ることで、regexp.関数も第2戻り値にエラーを返しますが、_で受け取ることでエラー処理を回避できます。
var whitespaceRegex, _ = regexp.Compile("\\s+")同氏は、
var whitespaceRegex = regexp.MustCompile(`\s+`)同氏は、varやinit()関数で初期化する場合などは、regexp.などを使用するべきだと主張していました。一方、errorを受け取って適切に処理すべきだと述べていました。
このように、Mustで始まる関数が用意されている場合があります。Mustで始まる関数は、errorを返す関数を呼び出し、errorがnilではない場合は、panicを起こすように定義されています。
また、raw string literalを使うべきだと指摘されていました。
deferにおけるエラー処理
以下のように、deferを使ってファイルをCloseする場合があると思います。deferを使えば、Closeを呼び出すことができます。
func run() error {
in, err := os.Open(*input)
if err != nil {
return err
}
defer in.Close()
out, err := os.Create(*output)
if err != nil {
return err
}
defer out.Close()
// some code
}このとき、defer in.のように閉じてしまっても問題ないですが、
func run() (err error) {
in, err := os.Open(*input)
if err != nil {
return err
}
defer in.Close()
out, err := os.Create(*output)
if err != nil {
return err
}
defer func() {
if cerr := out.Close(); err == nil {
err = cerr
}
}()
// some code
}deferの中で戻り値であるerrがnilである場合に、Closeで発生したエラーを代入することで、some code以下でエラーがあった場合は、err代入することでそのエラーを返すことができます。しかし、deferを使うと処理が複雑になることがあるため必ずしもdeferを使えば良いというわけではないとのことでした。上記の例では、some code以下がシンプルな場合はdeferを使わず、Closeを呼び出してエラーをチェックするほうがシンプルになると同氏は述べていました。
値とエラーを混ぜない
値としてエラーを使用している場合の例として以下のコードが挙げられていました。
func proc(it Iterator) (ret time.Duration) {
d := it.DurationAt()
if d == duration.Unterminated {
ret = -1
} else {
ret = d
}
// some code
}// duration.Unterminated = -1 * time.Second
func (it Iterator) DurationAt() time.Duration {
// some code
switch durationUsec := m.GetDurationUsec(); durationUsec {
case -1:
return duration.Unterminated
case -2:
return -2
default:
return time.Duration(durationUsec) * time.Microsecond
}
return -3
}このコードでは、time.は期間を表す型なので、error型で返すべきだと主張していました。
エラーの設計
エラー処理の方法によって、err != nilでエラーの有無をチェックする場合は以下のようにfmt.とerrors.を使用すると良いそうです。
fmt.Errorf("error in %s", val) もしくは errors.New("error msg")また、Err-が一般的なようです。
var (
ErrInternal = errors.New("foo: inetrnal error")
ErrBadRequest = errors.New("foo: bad request")
)error型はErrorメソッドを持つインターフェースです。そのため同氏は、Errorメソッドを実装することでerrorとして使用することができると述べていました。なお、-Errorという名前を使用するのが一般的らしいです。
type FooError struct { /* エラー情報のフィールド */ }
func (e *FooError) Error() string { return エラーメッセージ }
&FooError{ /* エラー情報 */ }同氏は、Errorメソッドのレシーバをポインタにした場合、nilとの比較に注意すべきだと指摘していました。インターフェースは値と型情報を持ち、nilの場合にnilとなります。そのため、if err != nil {がtrueとなります
import "log"
type FooError struct{}
func (e *FooError) Error() string { return "foo error" }
func foo() error {
var ferr *FooError // ferr == nil
return ferr
}
func main() {
err := foo()
if err != nil {
log.Fatal(err)
}
}また同氏は、panicは極力使わないようにすべきだと主張していました。どうしてもpanicを使いたい場合は、recoverを使ってパッケージ外に出るときはerrorにすべきだと述べていました。
インターフェースの埋込み
以下のコードはscan.インターフェースを埋め込むことでをColumnWriterがscan.インターフェースを実装していることを明示的にしようとしています。
// Column writer implements the scan.Writer interface.
type ColumnWriter struct {
scan.Writer
tmpDir string
// some other fields
}たしかにscan.を埋め込むことで、ColumnWriterはscan.を実装していることになります。しかし、scan.を埋め込む必要があるうえにこれではColumnWriterがscan.を本当に実装しているかどうかの保証にはなりません。同氏は、scan.型に代入することで、*ColumnWriterがscan.を実装しているかどうか、*ColumnWriterがscan.インターフェースを実装していない場合には、_で代入を受けているため、
// ColumnWriter is a writer to write ...
type ColumnWriter struct {
tmpDir string
// some other fields
}
var _ scan.Writer = (*ColumnWriter)(nil)なお、nilの場合）、panicが発生します。
import "fmt"
type I interface {
Key() string
Value() string
}
type S struct{ I } // SはIのメソッドをもつ
func (s S) Key() string { return "type S" }
func main() {
var s S
fmt.Println("key", s.Key())
fmt.Println(s.Value()) // runtime error: invalid memory address or nil pointer deference
}同氏は、
コードを見やすくする
構造体のフィールドのレイアウト
以下のように、muがどのフィールドを保護しているのかわからないと指摘していました。
type Modifier struct {
pmod *profile.Modifier
cache map[string]time.Time
client *client.Client
mu sync.RWMutex
}そのため、sync.はそれが保護しているフィールドのブロックの先頭に置くとわかりやすいとのことでした。
type Modifier struct {
client *client.Client
mu sync.RWMutex
pmod *profile.Modifier
cache map[string]time.Time
}長い行は簡潔な変数名を使って短く
以下のように、
package sampling
import (
servicepb "foo/bar/service_proto"
)
type SamplingServer struct {
// some fields
}
func (server *SamplingServer) SampleMetrics(
sampleRequest *servicepb.Request, sampleResponse *servicepb.Response,
latency time.Duration) {
// some code
}同氏は、grepすることを考えると1行にする方が良いと述べていました。
しかし同氏は、
samplingパッケージのSamplingServerは冗長である。Serverという名前を付ければ、sampling.となる。Server
- レシーバ変数は数文字でよい。
- 引数も型名から推測できる名前は付ける必要はなく、
短くて良い。 - 基本型の引数の場合は、
わかりやすい名前にする。 - ローカル変数も短く
（ indexよりi、readerよりrのほうが良い）。 - ローカル変数の名前を短くしても分かるように関数も小さくする。
上記の1行が長いコードは、
package sampling
import (
spb "foo/bar/service_proto"
)
type Server struct {
// some fields
}
func (s *Server) SampleMetrics(req *spb.Request, resp *spb.Response, latency time.Duration) {
// some code
}素直なコードフロー
インデントは最小にする
同氏は、ifの条件を入れ替えることでもっと簡潔に書けるそうです。
if _, ok := f.dirs[dir]; !ok {
f.dirs[dir] = new(feedDir)
} else {
f.addErr(fmt.Errorf("..."))
return
}
// some codeまた、okにする必要はなく、okだと不自然であると指摘していました。 この場合であれば、okの代わりにfoundという名前を使い、!okをfoundにすることができるとのことでした。
if _, found := f.dirs[dir]; found {
f.addErr(fmt.Errorf("..."))
return
}
f.dirs[dir] = new(feedDir)
// some code上記のコードのように、elseを使う必要がなく、
関数の分割
以下のコードでは、
func (h *RESTHandler) finishReq(op *Operation, req *http.Request, w http.ResponseWriter) {
result, complete := op.StatusOrResult()
obj := result.Object
if complete {
status := http.StatusOK
if result.Created {
status = http.StatusCreated
}
switch stat := obj.(type) {
case *api.Status:
if stat.Code != 0 {
status = stat.Code
}
}
writeJSON(status, h.codec, obj, w)
} else {
writeJSON(http.StatusAccepted, h.codec, obj, w)
}
}そこで、finishStatus）
func finishStatus(r Result, complete bool) int {
if !complete {
return http.StatusAccepted
}
if stat, ok := r.Object.(*api.Status); ok && stat.Code != 0 {
return stat.Code
}
if r.Created {
return http.StatusCreated
}
return http.StatusOK
}
func (h *RESTHandler) finishReq(op *Operation, w http.ResponseWriter, req *http.Request) {
result, complete := op.StatusOrResult()
status := finishStatus(result, complete)
writeJSON(status, h.codec, result.Object, w)
}switchを使う
以下のコードでは、"small"、"medium"、"large"、"null"）
func BrowserHeightBucket(s *session.Event) string {
browserSize := sizeFromSession(s)
if h := browserSize.GetHeight(); h > 0 {
browserHeight := int(h)
if browserHeight <= 480 {
return "small"
} else if browserHeight <= 640 {
return "medium"
} else {
return "large"
}
} else {
return "null"
}
}このコードでは、ifが入れ子になっていて、if elseによって長くなっています。 同氏は、ifではなくswitchを使うと以下のように簡潔に書けると述べていました。
func BrowserHeightBucket(s *session.Event) string {
size := sizeFromSession(s)
h := size.GetHeight()
switch {
case h <= 0:
return "null"
case h <= 480:
return "small"
case h <= 640:
return "medium"
default:
return "large"
}
}シンプルに実装する
time.Duration を使う
同氏は、int型ではなく、time.型を使うべきだと指摘していました。また、flag.関数を使うことでtime.型の値を取得できると述べていました。
var rpcTimeoutSecs = 30 // Thirty seconds以下の2つの例では、time.型を使用していますが、time.と掛け算をするとその結果はtime.型となります。
var rpcTimeout = time.Duration(30 * time.Second) // Thirty secondsvar rpcTimeout = time.Duration(30) * time.Second // Thirty seconds同氏は型変換をなくすと、30 * time.で十分意味が通じるので、// Thirty secondsのような冗長なコメントは必要ないと指摘していました。
var rpcTimeout = 30 * time.Secondチャネルを使う
同氏は、
type Stream struct {
// some fields
isConnClosed bool
connClosedCond *sync.Cond
connClosedLocker sync.Mutex
}
func (s *Stream) Wait() error {
s.connClosedCond.L.Lock()
for !s.isConnClosed {
s.connClosedCond.Wait()
}
s.connClosedCond.L.Unlock()
// some code
}
func (s *Stream) Close() {
// some code
s.connClosedCond.L.Lock()
s.isConnClosed = true
s.connClosedCond.L.Unlock()
s.connClosedCond.Broadcast()
}
func (s *Stream) IsClosed() bool {
return s.isConnClosed
}StreamがCloseされているかどうかをisConnClosedで管理し、sync.とsync.を使って制御しています。同氏は、
type Stream struct {
// some fields
cc chan struct{}
}
func (s *Stream) Wait() error {
<-s.cc
// some code
}
func (s *Stream) Close() {
// some code
close(s.cc)
}
func (s *Stream) IsClosed() bool {
select {
case <-s.cc:
return true
default:
return false
}
}上記のチャネルを使ったコードでは、Streamが開いているか閉じているかに対応させています。Closeメソッドはチャネルを閉じるだけで実現しています。そして、Waitメソッドは単にチャネルから受信すればよく、IsClosedメソッドは、selectのcase <-s.のケースは実行されず、defaultのケースが実行されます。一方、case <-s.のケースが実行されます。
型がわかっている場合にreflectは使わない
同氏は、reflectパッケージは強力であり、reflectを使用しているとのことでした。
type Layers struct {
UI, Launch /* more fields */ string
}
layers := NewLayers(s.Entries)
v := reflect.ValueOf(*layers)
r := v.Type() // type Layers
for i := 0; i < r.NumField(); i++ {
if e := v.Field(i).String(); e != "-" {
eid := &pb.ExperimentId{
Layer: proto.String(r.Field(i).Name()),
ExperimentId: &e,
}
experimentIDs = append(experimentIDs, eid)
}
}上記のコードの場合、reflectの対象のLayersがどのような型であるかコンパイル時にわかっているため、reflectを使う必要はないと同氏は指摘していました。そして、SliceというLayerとExperimentの対のスライスを返すメソッドを追加することで、reflectを使うよりも意図がわかりやすくなると主張していました。
type LayerExperiment struct{ Layer, Experiment string }
func (t *Layers) Slice() []LayerExperiment {
return []LayerExperiment{
{"UI", t.UI},
{"Launch", t.Launch},
/* more fields */
}
}
layers := NewLayers(s.Entries).Slice()
for _, l := range layers {
if l.Experiment != "-" {
eid := &pb.ExperimentId{
Layer: proto.String(l.Layer),
ExperimentId: proto.String(l.Experiment),
}
experimentIDs = append(experimentIDs, eid)
}
}テスト
同氏は可読性のレビューにおいて、t.メソッドで入力と期待する出力と実際の出力をメッセージとして出すものであると述べていました。このように書いておけば、
// 典型的なテストコード
if got, want := テスト対象(input), 期待値; !テスト(got, want) {
t.Errorf("テスト対象(%v) = %v; want %v", input, got, want)
}なお、
func ExampleWrite() {
var buf bytes.Buffer
var pi float64 = math.Pi
err := binary.Write(&buf, binary.LittleEndian, pi)
if err != nil {
fmt.Println("binary.Write failed:", err)
}
fmt.Printf("% x", buf.Bytes())
// Output: 18 2d 44 54 fb 21 09 40
}コメント
レビューにおいて、
- パッケージコメントを書く
（ mainパッケージはコマンドのコメント） - エクスポートしているものにはコメントを付ける
- コメントは対象としているものの名前から始まる文にする
を挙げていました。
上記の3つの項目を満たす正しいコメントは以下のようになるそうです。
// Package math provides basic constants and mathematical functions.
package math
// A 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) {このように書いておくと、grepもしやすくgodocで確認した場合も見やすくなるとのことでした。また、godocは-httpオプションをつけてWebブラウザで確認することもできると述べていました。
$ godoc bytes Buffer $ godoc -http=:6060 # http://localhost:6060/pkg
同氏は、godocで確認することで、
APIデザイン
同氏は、utilというパッケージを作ってそこにいろいろなものを入れてしまいがちだが、
同氏はAPIをシンプルにするためのルールとして以下の項目を挙げていました。
- 複数の戻り値を使えるので、
出力変数としてポインタは使わない。 - スライス、
マップ、 チャネル、 インターフェースへのポインタは使わない。 - エラーはerrorで返す
（panicは使わない）。 - 一般的なインターフェース(fmt.
Stringer 、io.Reader など)に挙動が適合する場合は合わせる。 - 引数などはインターフェースのほうが組み合わせやすいしテストもしやすい。
- 非同期APIより同期API
（パッケージを越えてチャネルあまり使わない）。 - 非同期にするにはAPIを使う側がゴルーチンとチャネルで制御する。
まとめ
同氏は、
- 適切な名前を選ぶ
- シンプルなAPIを提供する
- わかりやすいドキュメントを書く
- 複雑にしすぎない
この基調講演では、