GoのWebアプリで見かけたツラいコード

構造体のフィールドにContextを持たせる

ほとんどの場合、各メソッドの引数にいちいちctxを渡すのが面倒だという理由だけで以下のようにしている印象がある。

type S struct {
    ctx context.Context
}

func (s *S) A() {
    // s.ctxを使う
}

func (s *S) B() {
    // s.ctxを使う
}

func (s *S) C() {
    // s.ctxを使う
}

ちょっとくらいタイプ数を減らすよりも素直に引数で渡すようにした方がシンプルだし、将来的に変にContextを共有するようなコードになってしまうのも防げる。

func (s *S) A() {
    value := getValue(s.ctx)
    s.ctx = context.WithValue(s.ctx, "key", value)
}

func (s *S) B() {
    // 事前にA()を呼んでおく必要がある?
    value := s.ctx.Value("key")
}

https://blog.golang.org/context-and-structs にも書いてあるように、どうしてもそうしなければいけない理由がない限りはやらない方が良い。

Contextの中に参照を入れておいて任意の場所で更新する

GoのContextはcontext.WithValueで新しいものを作って呼び出し先に渡す形になっているため、基本的に呼び出し元は呼び出し先の影響を受けることがない。
ただ、予めContextの中に参照を入れておくと呼び出し元に影響を与えることができてしまう。

func f(ctx context.Context) {
    // この段階では ctx.Value("key").(*V).value が空
    do(ctx)
    // ctx.Value("key").(*V).value の値が変化
}

構造体でContextを共有するのと組み合わせると非常に危険。

関連が把握しきれなくなると直すに直せなくなってしまうので、こういうコードはなるべく書かない。
もしくはせめて影響範囲を限定できるようにしておきたい。

似ている処理を匿名の構造体でまとめる

https://golang.org/doc/effective_go#embedding のようにすることでGoでも継承のようなことができる。
しかし、何でもかんでもこれを適用してしまうと扱いにくいコードになってしまう可能性がある。

type Common struct {
    req *http.Request
    rw  http.ResponseWriter

    id   int64
    data map[string]interface{}
}

func (c *Common) Prepare() {
    c.id = idFromPath(c.req.URL)
    c.data = decodeBody(c.req.Body) // GETリクエストの場合は不要
}

func (c *Common) ID() int64                    { return c.id }
func (c *Common) Data() map[string]interface{} { return c.data }

type Handler struct {
    Common
}

func (h *Handler) Get() {
    // Trace系の処理をしたりとか

    h.Prepare()

    res := getResource(h.ID())
    // 続く...
}

func (h *Handler) Put() {
    // Trace系の処理をしたりとか

    // Prepare忘れ!

    res := putResource(h.ID(), h.Data())
    // 続く...
}

この程度ならまだわかりやすいが、継承が多段になったり、レイヤやパッケージや処理がさらに細分化されていくことで、だんだんとわかりにくいコードになっていってしまう。

そのため、共通処理は親クラスのメソッドではなく関数を使うようにするなど、なるべく暗黙的な要素を排除しておいた方が後になって困ることが少ない。

特に状態が変化する構造体を埋め込む際は要注意。