毎日他の人のコミットをながめる文化で生活していると、理由は浮かばないけど「ん？このコミットはなんか気になる」と感じるようになります。それは、新しいことを知ることができたコミットだったり、真似したくなるようなコードが入っているコミットだったり、なんかまずそうな気がするコミットだったり、様々です。

「ん？」と感じてコミットを見直してみても、何が気になったか自分でもすぐにわからない場合があります。そんなとき、気になったことをコミットした人に伝えるために、コミットへのコメントをまとめ始めます。「コミットした人に伝えるため」というように、他の人に伝えようとすることがポイントです。他の人に伝えるためにまとめようとすると、思いの外なにが気になったかまとまるものです。

今回は、メタプログラミングを使ってコードを整理したコミットで「ん？」と感じたときのことについて紹介します。このおかげで「メタプログラミングをして割に合うかの判断基準」を1つ明確にできました。その基準とは「処理を1箇所に局所化できるか」です。

該当コミットとコメント

このとき「ん？」と感じたコミットはdroonga/fluent-plugin-droonga@1b22308です。

もともと次のようなコードがありました。

def format
  formatted_result = {}

  if need_element_output?("count")
    format_count(formatted_result)
  end

  if need_element_output?("attributes")
    format_attributes(formatted_result)
  end

  if need_element_output?("records")
    format_records(formatted_result)
  end

  if need_element_output?("startTime")
    format_start_time(formatted_result)
  end

  if need_element_output?("elapsedTime")
    format_elapsed_time(formatted_result)
  end

  formatted_result
end

これを次のように整理しています。フォーマット対象すべてをHashにしてまとめて扱えるようにし、フォーマットする処理を動的に変えるコードにすることで同じようなコードを1つにまとめています¹。

SUB_FORMATTERS = {
  "count"       => :format_count,
  "attribtues"  => :format_attributes,
  "records"     => :format_records,
  "startTime"   => :format_start_time,
  "elapsedTime" => :format_elapsed_time
}

def format
  formatted_result = {}

  SUB_FORMATTERS.each do |name, sub_formatter_method_name|
    if need_element_output?(name)
      method(sub_formatter_method_name).call(formatted_result)
    end
  end

  formatted_result
end

25行あったコードが19行になり、何度もでてきた次のようなコードのパターンがなくなっています。

if need_element_output?(NAME)
  format_NAME(formatted_result)
end

整理されているように見えます。しかし、「ん？」と感じたのです。

コメントをまとめてわかったこと

どうして「ん？」と感じたことを伝えるために、コミットにコメントしました。このコメントを書き始めたときはどうして「ん？」と感じたかわかっていなかったのですが、コメントを書きながらわかってきました。わかったことを紹介します。それは、「メタプログラミングをして割に合うかの判断基準」です。

メタプログラミングをすれば動的にプログラムを実行することができます。プログラムの一部をパラメーター化することでコードの重複を取り除くこともできます。うまく使えばリーダブルにもなります。しかし、使い方によっては理解しにくくなったり、メンテナンスしにくくなったりします。メタプログラミングはうまく付きあうことが難しい機能です。

このコミットで見たメタプログラミングの使い方は割に合わないと感じました。

オーバースペックなメタプログラミング

このケースではメタプログラミングを使わず、次のようにcase whenで十分ではないかと感じました。

def format
  formatted_result = {}

  @request.output["elements"].each do |name|
     case name
     when "count"
       value = format_count
     when "attributes"
       value = format_attributes
     when "records"
       value = format_records
     when "startTime"
       value = format_start_time
     when "elapsedTime"
       value = format_elapsed_time
     else
       next
     end
     formatted_result[name] = value
    end
  end

  formatted_result
end

# 参考: formatメソッドで使っていたneed_element_output?の実装イメージ
# def need_element_output?(name)
#   @request.output["elements"].include?(name)
# end

case whenの書き方では行数は24行ともともとの25行とほとんど変わりませんが、重複していた次のパターンは解消しています。

if need_element_output?(NAME)
  format_NAME(formatted_result)
end

どうしてこれで十分なのか。それは、メタプログラミングを使うようにしても、新しくフォーマット対象を追加する時の変更箇所を局所化できていないからです。

メタプログラミングを使うようにした場合、新しくフォーマット対象を追加するときは次のように2箇所変更します。

• format_XXXというフォーマットメソッドを定義
• 別の場所にあるSUB_FORMATTERSに定義したメソッドを登録

変更箇所を局所化できていないと、このあたりのコードをいじるたびに、「このメタプログラミングをしているところはどういう仕組みで動いているか」を理解しなければいけません。これは通常のコードを理解するよりも高くつきます。抽象化のレイヤーにヒビが入っていて、違うレイヤーがのぞいてしまっているイメージです。

よって、変更箇所を局所化できるほど抽象化できていないメタプログラミングのコードはオーバースペックと感じていることがわかりました。それなら、メタプログラミングではなくてcase whenで十分だと感じたということです。

逆に言うと、メタプログラミングでやっていることの詳細を知らなくても使えるくらい抽象化しているのであれば、メタプログラミングのメリットを感じているということです。今回のケースだと例えばこんな感じ、というものはコミットへのコメントを参照してください。

その後、このあたりのコードはdroonga/fluent-plugin-droonga@6943d69というようになりました。case whenベースで前述のコードよりさらにスッキリしたものです。

まとめ

「ん？」と感じたコミットに対してコメントをまとめることで「メタプログラミングをして割に合うかの判断基準」の1つが明確になりました。判断基準は次の通りです。

• 処理を1箇所に局所化できるか

言いかえると、こうなります。

• メタプログラミングでやっていることの詳細を知らなくても使えるくらい抽象化しているか

このように、自分がどのように感じているか自分でも曖昧なことを明確にできるので、次のことをオススメします。

• 普段から他の人のコミットをながめる生活を当たり前にする
• 「ん？」と感じるコミットがあったらどうして自分がそう感じたかをコミットした人に伝わるようにまとめる

これができるようになるお手伝いをしています。興味のある方はコミットへのコメントサービスを参照してください。

ツイートする

フォローする