概要

この記事はフラー Advent Calendar 2020 の 11日目の記事です。
10日目は、@seto_inugamiさんで、 Androidで毎秒カウントダウンをするappWidgetを作ってハマりまくった話でした。

こんにちは。ふみおです。

プログラマに転職して早くも9ヶ月が経ち、現在、サーバサイドエンジニアとして案件にアサインされています。

githubもの使い方もわからなかった自分が、今となってはAPIの実装をさせてもらっています。　　
そこで、今回は今までのコードレビューで指摘されたことをまとめてみようと思います。

自分が再度同じミスをしてないか。指摘されたことを忘れてないか。を再認識するためにもまとめていこうと思います。
プログラミング言語はGo言語になります。

1. 日本語について

まず1番印象にある指摘事項が日本語です。今も日本語の指摘をいただくことがあります。
今までにこのような指摘を頂きました。
・1文は50文字程度に収める
・接続詞で1文にまとめる必要があるのか。それとも2文で表しても問題ないのか
・読みやすい文章の句読点の数
・何が主語かを明確に示す
・言葉の並びによっては複数の読み取り方ができてしまう
・用語は統一する etc...
日本語、難しいです。

ここまで丁寧に指摘してくれることは今後の社会人生活でもそうないと思うので、今のうちに適切な日本語を書けるよう気をつけています。

頂いた指摘を受けて、以下のことを意識しています。
・自分で書いた説明に誤字脱字、過不足がないかセルフレビューする
・自分は全くその説明に対して無知な人である想定をして文章を読む
・端的な文章にするために、無駄な文はないか確認する

誰が読んでも理解できるような簡潔な文章であるか？を大切にしています。
就活の志望動機を書いているかのような気持ちで書いています。

昔から本を読む人ではなく、文章の構成や書き方が下手なので参考書を読み始めました。
理科系の作文技術

こちらは文章の組み立て方や、簡潔な表現の仕方などが記されています。

2. Goの書き方について

趣味でプログラムを書いていた時は、とりあえず動くプログラムを作ることしか気にしていなかったのですが、業務のプログラムとなると全く異なるなと感じました。
業務のソースレビューにて頂いたGoを書く上での指摘事項を書き連ねたいと思います。

2-1. ポインタ型は`nil`チェックを行う

ポインタ型の変数を呼び出す前に、その変数がnilではないことを確認する必要があります。
ポインタ型はnilを許容するためです。
nilである変数の中身を読み込もうとしても読み込めないため、panicして処理が中断されてしまいます。

package main

import (
    "fmt"
)

type Temp struct {
    str *string
}

func main() {
    pnt := &Temp{}
    fmt.Println(*pnt.str)  // panic: runtime error: invalid memory address or nil pointer dereference
}

nilチェックの仕方は以下のように書きます。

func main() {
    pnt := &Temp{}
    if pnt.str != nil {
        fmt.Println(*pnt.str)
    }
}

2-2. スライスの容量が推測できる時は最初に設定する

Go 言語においては、 sliceという可変長配列が存在します。
make関数でスライスを初期化できます。
初期化時に、スライスの容量(capacity)が事前に推測できるときは、初期化するタイミングで容量を設定したほうが良いです。
スライスのの要素数(length)は容量を超えることはでき無いので、事前に容量を推測できるときは容量を確保したほうがいいのです。

array := make([]int, length, capacity)

2-3. `if-else`文をシンプルに実装する

複雑なif-else文の実装の際、条件が入れ子になりすぎてしまったりすることがあります。
それを避けるために、実装可能なのであれば、else文は使わずにif文だけで実装できた方がより綺麗に見えます。
この内容はGoのよりベターな実装方法を解説したGo Code Review Commentsにも記載があります。
https://github.com/golang/go/wiki/CodeReviewComments#indent-error-flow

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

ではなく、

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

のように条件分岐が少ないように実装するよう記載されています。

2-4. 略語の変数名の付け方

IDやURL、HTTPといったワードを変数名に使うことがあると思います。
その際は、Id、Urlというように頭文字だけ大文字で定義することはさけるべきです。
これらの単語は一般的に頭文字を取って略されているため、すべて大文字で定義するのが最もわかりやすいです。
上記の例以外に以下のようなものも該当になります。
ex. XML、NG、API などなど

こちらの内容もGo Code Review Commentsも記載があります。
https://github.com/golang/go/wiki/CodeReviewComments#initialisms

2-5. 時間や数値を含むテストは境界値テストケースも実装する

ある値を境目で、処理結果が変わるときは境界値テストを行いましょう。
例えば、 2020-10-01 00:00:00 +0000 UTCの時刻を境目とした時、
境界値テストケースとしては

2020-10-01 00:00:00 +0000 UTC以降の時間
と
2020-09-30 23:59:59 +0000 UTC以前の時間
でテストケースを設けるべきです。

2-7. 適切なステータスコードを設定する

4XX系もしくは5XX系のどちらで返すのが適切か指摘されたことがあります。
当初はそこまで深く考えずステータスコードを設定していましたが、サーバ側では4XX系、5XX系の間で監視設定に差があることもあります。
監視で拾えるエラーも不適切な設計によっては見落としてしまうことがあります。

また、API開発においてベンダーAPIを介すことは多くはないと思います。
・ベンダーのAPIの実装内容を全て理解しているわけではない
・どのようなエラーレスポンスが返ってくるかわからない
・仕様書にすべてのエラーケースが記載されていない
・ベンダーAPIのレスポンスが想定できない

などもあり、クライアントに誤ったエラーレスポンスを返してしまうこともあったりしました。
それから、事前にAPIの挙動を知りつくすためにレスポンスの確認を行うようにしています。

他にもまだまだあるのですが、印象に残った指摘事項をまとめてみました。
こうやって振り返ることで、抜けかけていた意識を取り戻せるのでいい試みだったと思います。
定期的に振り返っていこうと思います。