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()) // 続く... }
この程度ならまだわかりやすいが、継承が多段になったり、レイヤやパッケージや処理がさらに細分化されていくことで、だんだんとわかりにくいコードになっていってしまう。
そのため、共通処理は親クラスのメソッドではなく関数を使うようにするなど、なるべく暗黙的な要素を排除しておいた方が後になって困ることが少ない。
特に状態が変化する構造体を埋め込む際は要注意。