RubyOnRailsで開発していると、よく無駄に長いコードや読みにくいコードを書いてしまうことがあります。
私もそんな失敗経験があるため、それを自分へのメモがてら書き留めていこうと思います。
今回はよくあるBlogテーブルとCommentテーブルを例に説明していきます。
リファクタリング 1st
true/falseの条件分岐でif,else,endを使ってしまう。
before
Class Blog < ActiveRecord::Base
has_many :comments
def has_comments?
if self.comments.count > 0
true
else
false
end
end
end
長いです！長いです！長いです！こんなコードに５行も要りません！１行に圧縮してしまいましょう！
after
Class Blog < ActiveRecord::Base
has_many :comments
def has_comments?
self.comments.count > 0
end
end
ちなみに、間違っても下記のような無駄なコードは書いてはいけません。気をつけましょう！
Class Blog < ActiveRecord::Base
has_many :comments
def has_comments?
self.comments.count > 0 ? true : false
end
end
リファクタリング 2nd
@blog.commentsなどの関連モデル全てが同じ条件を満たすかどうかの判定を書くとき。
before
Class Blog < ActiveRecord::Base
has_many :comments
belongs_to :user
def only_my_comments?
return_value = true #default
self.comments.each do |comment|
if comment.user_id != self.user_id
return_value = false
end
end
end
end
悪くはないのですが、やはり長いですね。これも１行に圧縮してしまいましょう。
after
Class Blog < ActiveRecord::Base
has_many :comments
belongs_to :user
def only_my_comments?
self.comments.all?{|comment| comment.user_id == self.user_id}
end
end
リファクタリング 3rd
わりとよくある、present?メソッドを使って条件分岐させるパターンです。
before
# erb風でのview表記
<% if @blog.present? %>
<%= @blog.name %>
<% else %>
<%= "No title" %>
<% end %>
よく使う分、こういうコードを書かれうと玄人の方々にとっては少し頭を悩ませてしまいます。下記のようにリファクタリングしてしまいましょう。
after
<%= @blog.try(:name) || "No Title" %>
リファクタリング 4th
eachメソッドでループをまわして、それぞれの合計値を取るというパターンです。数量や価格やイイネ数などを取るときによく使われます。
before
all_like_count = 0
@blogs.each do |blog|
all_like_count += blog.like_count
end
このリファクタリングで活躍するのがmapメソッドとinjectメソッド。ある程度こなしているプログラマーならばこういうリファクタリング済みコードを見るとテンションがあがる。
after
all_like_count = @blogs.map(&:like_count).inject(:+).to_i
上記で最後に.to_iメソッドを使っているのですが、人によってはこんなのいらないという人もいるかもしれません。しかし、@blogs = nilのときはall_like_count = nilになってしまいます。一見無駄に見えますが必要なコードなので忘れないようにしましょう。
最後に
実際にRailsアプリケーションを書いていると、Routingまわりやhelper周りなどでもたくさんの改善ポイントがあったりします。ですがとりあえず今回は１行に集約できるリファクタリングのみとして４tipsまでにとどめ提供してみました。楽しいプログラミングに役立つことができれば幸いです。