132
121

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 5 years have passed since last update.

Ruby on RailsAdvent Calendar 2014

Day 22

RubyOnRailsでこんなコードを書いてたら今すぐリファクタリングして1行にまとめちゃおうぜ!small tips!

Last updated at Posted at 2014-12-19

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

長いです!長いです!長いです!こんなコードに5行も要りません!1行に圧縮してしまいましょう!

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

悪くはないのですが、やはり長いですね。これも1行に圧縮してしまいましょう。

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周りなどでもたくさんの改善ポイントがあったりします。ですがとりあえず今回は1行に集約できるリファクタリングのみとして4tipsまでにとどめ提供してみました。楽しいプログラミングに役立つことができれば幸いです。

132
121
4

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
132
121

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?