2012年9月15日土曜日

コードの重複を排除するのもほどほどに!

コードの重複を排除する、ということは良いコードを書くための指針として昔から言われている。全く同じ構造のコードを1000行にわたって繰り返すことがばかげていることは誰にだってわかる。ruby はそのようなコードの重複を排除することに優れた言語だと思う。メタプログラミングを工夫することでかなり重複を排除することができる。

例えば

def opened?
  self.state == "opened"
end

def closed?
  self.state == "closed"
end

def waiting?
  self.state == "waiting"
end

のようなコードもメタプログラミングを使えば

%w(opened closed waiting).each do |name|
  define_method "#{name}?" do
    self.state.to_s == name
  end
end

のように書くことができる。

これはプログラマとしてはなかなか楽しい作業となる。コードの重複した部分を見つけて、うまいことメタプログラミングを使うことで、ちょっとしたパズル感覚で、しかもコードがぐっと短くなる。とても楽しい。

だから僕らはついついやりすぎてしまう。これが問題だ。コードの重複の排除というのは局所最適化でしかない。局所最適化を繰り返すことで全体最適化が達成されるというのは世界のほぼ全ての場所で成立しない。局所最適化のやりすぎは全体最適化を損なうというのは一般的な法則と言ってよい。

短いコードは良いコードだ。だがそれをどこまでも追及するとわけのわからんコードができる。短いコードが本当にいいコードなら、zip 圧縮してそれを読めばよい。

コードの重複を排除する、というルールをやりすぎると逆に読みづらいコードが出来上がる。重複排除がコードの品質の悪化につながるのはどういう場合か?重複の排除というのは通常同じ構造の部分をメソッドなりブロックなどで切り出すことで達成される。その切り出したコードがビジネスロジック上、あるいはアルゴリズム上などで明確で簡潔な名前をつけることができない場合、コードの品質は下がる。

機械的に重複部分だけをぴったり切り出した場合、そのようなことがよくおきる。そのようなメソッドにはよくわからないメソッド名がつけられてしまい、後からメソッドを読んだ時にそのメソッドが担っている役割が読み取りづらく、結果記憶もしづらい。メソッド名を読んでも何をしてるものなのかわからないから、結局コードの中身を一行一行読む、ということがおき、しかも繰り返す。無理やりわかりやすいメソッド名を付けても、それが内容とかい離しているようなら誤解を招くだけでより悪い。

だから簡潔なビジネスロジック上意味のある単位でコードを切り出すというのはとても重要なルールとなる。たとえそういうい切り出しによってすべての重複が排除できなかったとしても、それは気にしなくてよい。メソッドにはわかりやすい名前をつけよう、とよく言われているが、それは順序が逆なのだと思う。わかりやすい名前をつけることができるメソッドの切り出し方にしよう、というのが本来あるべき姿のはずだ。

このおかしさをわかってもらうためにしつこく説明を繰り返すが、コードから機械的に重複部分を切り出して、そこにドメインの単語をつける、というのはあってはならない。前者はシンタックスで後者はセマンティクスだ。表現すべき意味がまずあってそれをコードで表現しているはずなのに、そのコードを機械的に意味を考えずに切り貼りして、そのコードの断片がドメイン上偶然意味を持つか?という話。

具体例が出せるともっと伝わるんだろうな。もしコード書いてて具体例に出会ったらまた追記します。

0 件のコメント:

コメントを投稿