Rails でよくある 5 つの間違い - 杉風呂2.0

以下の文章は、Mike Perham 氏のブログ記事を翻訳したものです。原文は2012年5月5日に公開されました。

Five Common Rails Mistakes
http://www.mikeperham.com/2012/05/05/five-common-rails-mistakes/

Railsをそれなりに仕事で使ってきて多くの Rails アプリを見てきて、悪い Ruby コードを読み書きした。この記事では、だいたいすべての Rails のコードベースで見られる共通の間違いを5つ挙げる。

1. スキーマの仕様がないマイグレーション
データモデルはアプリケーションのコアだ。スキーマに制約がないと、データはコードベースに存在するバグにより徐々に蝕まれていき、フィールドに値が入っているか信頼できなくなる。ここに Contact スキーマがあるとしよう。

create_table "contacts" do |t|
  t.integer  "user_id"
  t.string   "name"
  t.string   "phone"
  t.string   "email"
end

これに何が必要だろうか？おそらく、User に所属(belogs_to)していて、1つの名前を持っているだろう。データベースの制約を使ってこれを保証しよう。:null => false を追加してやれば、バリデーションにバグがあったとしても、データベースがモデルの保存を許さないので、モデルが常に一貫性を保てる。

create_table "contacts" do |t|
  t.integer  "user_id", :null => false
  t.string   "name", :null => false
  t.string   "phone"
  t.string   "email"
end

ボーナスポイント
limit => N を使って、string 型のカラムを大きさを正確に決めてやろう。 string はデフォルトで255文字で、電話番号にはそれだけの大きさは必要がない。

2. オブジェクト指向プログラミング
ほとんどの Rails プログラマはオブジェクト指向の Ruby コードを書いていない。MVC 指向の Ruby コードを書いてモデルとコントローラを所定の場所に配置する。たいていはユーティリティモジュールにクラスメソッドを書いて lib/ に追加する。2、3年して開発者は気づく。「 Rails も Ruby なんだ。Rails が明示して保証していないやり方でも、シンプルなオブジェクトを作って組み立てていいんだ！」と。

ボーナスポイント
サードパーティのサービスを呼び出すのにファサードを導入しよう。テストコードにモック・ファサードを提供し、テストスイートではサードパーティのサービスを実際に呼ばないようにできる。

3. ヘルパーで HTML を結合する
ヘルパーメソッドを作っていれば、最低限ビュー層をきれいに(clean)しようとしているということなので、賞賛しよう。しかし、開発者はヘルパー内部でタグを作る際の基本を知らないことが多く、文字列をきたなく結合したり改変したりすることになる。

str = "<li class='vehicle_list'> "
str += link_to("#{vehicle.title.upcase} Sale", show_all_styles_path(vehicle.id, vehicle.url_title))
str += " </li>"
str.html_safe

ああ、これは醜く、XSS のセキュリティホールを容易に生んでしまう！content_tagこそが友達だ。

content_tag :li, :class => 'vehicle_list' do
  link_to("#{vehicle.title.upcase} Sale", show_all_styles_path(vehicle.id, vehicle.url_title))
end

ボーナスポイント
ブロックをとるヘルパーメソッドを導入しよう。ネストしたブロックはネストした HTML に違和感なく合う。

4. すべてをメモリにロードする巨大なクエリ
データを訂正する必要がある場合、すべてをイテレータで回して修正するのではないかな？

User.has_purchased(true).each do |customer|
  customer.grant_role(:customer)
end

100 万ユーザを抱える EC サイトを運営していた場合、1 ユーザが 500 バイトだったらどうだろう？このコードはランタイムのメモリに 500MB を使っってしまう！より良い書き方はこうだ。

User.has_purchased(true).find_each do |customer|
  customer.grant_role(:customer)
end

find_each は find_in_batches を使っていて、一度に引っ張ってくるのは 1000 レコードなので、ランタイムのメモリ要件を劇的に下げてくれる。

ボーナスポイント
update_all か生の SQL を使って、大量更新のパフォーマンスを上げる。 SQL の学習にけっこうな時間がかかるが、それだけ利点は数多くある。パフォーマンスを 100 倍改善できるだろう。

5. コードレビュー！
Github を使っていると思うが、pull request は使っていないと思う。1、2日使って機能を追加するなら、ブランチ上で作業してpull request を送ろう。チームが君のコードをレビューできるようになり、改善点と考えていなかったエッジケースの提案が得られる。そのおかげで君のコードがより高い品質を持つことを保証する。僕らは TheClymb の変更の90%を pull request で行っていて、それらは 100% ポジティブな体験だった。

ボーナスポイント
最低限、ハッピーパス(happy path)ぐらいはテストコードなしで pull request をマージしたりしないこと。テスティングはアプリケーションを安定させて、平穏に眠るためには不可避だ。

僕がよくある問題を見逃していたらコメントで知らせて！

更新: Mike Perham 氏から許諾頂きました。ロシア語にも翻訳されていました。