【Rails版】半年で学んだより良いコードを書くためのTips10選

はじめに

半年で学んだシリーズになります。
前回は汎用的なことについて書いたつもりなので、こちらはよりRailsに寄せました。

前回の記事はこちら↓

また気になる点はぜひコメントいただけますと嬉しいです。
それでは、いきましょう！

Ruby on RailsのコードをよくするTips10選

1. boolean型を返す関数の末尾に?をつける
2. boolean型が入っている変数はis_で始める
3. 基本関連付けを使ってデータを取得する
4. バリデーションのテストは書かない
5. ActiveRecordはデフォルトでID同士を比較する
6. mapメソッドに置き換えよう
7. 展開してスッキリ書く
8. メソッド名に関するその他慣習
9. FatControllerを解消する
10. FatModelを解消する

1. boolean型を返す関数の末尾に?をつける

タイトル通りで、boolean型を返す関数の末尾には?をつけた方がわかりやすいです。

例

def adult?(age)
  age >= 18
end

実は初めての仕事で先輩さんに教えてもらったことです。

Railsチュートリアルを終えて、いざ！って感じだったので実務のレベルとは程遠いことを悟りました。
なので、たまに「すべてはここから始まったのか...」と思います。笑

2. boolean型が入っている変数はis_で始める

こちらもタイトル通り、boolean型が入っている変数はis_〇〇という命名にすることです。

例

is_hidden = true
is_new_task = false

ただ、Rubyの命名ではis_は避けることが多いみたいです。
「ん〜どっち？」と今でも思っています。

なので、チーム内で統一されていたらOKかと思って使っています。

3. 基本関連付けを使ってデータを取得する

Railsでは、基本的に関連付けを使ってデータなどを作成しましょう。

userに関連付けがされているGoalモデルを例に解説します。

Badな例

class GoalsController < ApplicationController
  def create
    @goal = Goal.new(goal_params)
    @goal.user = current_user

    if @goal.save
      ...
  end

Goodな例

class GoalsController < ApplicationController
  def create
    @goal = current_user.goal.build(goal_params)

    if @goal.save
      ...
  end

関連付けを使って取得する方が、セキュリティ的にも良かったりします。
なので、大文字のTaskやGoalなどが出現したらようチェックするようにしましょう。

4. バリデーションのテストは書かない

Railsにおいて、バリデーションのテストを書く必要はありません。
なぜなら、バリデーションはActiveRecordの機能なので、ActiveRecord側でテストすべきと考えるからです。

不要なテスト

# models/user.rb
class User < ApplicationRecord
  validates :name, presence: true
end

# models/user_spec.rb
RSpec.describe User do
  describe 'バリデーション' do
    context '名前が空文字の場合' do
      it 'エラーになる' do
        # 不要なテストケース
      end
    end
  end
...

つまり、二重で書くことになるため、冗長なわけですね。
同じ理由から、gemなどのライブラリをテストする必要もありません。

5. ActiveRecordはデフォルトでID同士を比較する

ActiveRecord、つまりモデルのオブジェクトはデフォルトでID同士を比較してくれます。
下記の例はタスクを作成したユーザーがログインしているユーザーかチェックしているコードです。

Badな例

if @task.user.id == current_user.id
...
end

Goodな例

if @task.user == current_user
...
end

Badだと少し冗長なので省略してかける方がベターだと思います。

6. mapメソッドに置き換えよう

そのコード、mapメソッドに置き換えれるよ〜という話です。
具体的には、

1. 配列を作る
2. 配列の中に要素を入れる
3. 配列を返す

をしているメソッドなどはmapメソッドに置き換えましょう。

下記のようなコードです。

mapメソッドに置き換えれるコード

def fetch_details(files)
  details = []
  files.each { |file| details << calc_word_counts(file) }

  details
end

mapメソッドを使ったコード

def fetch_details(files)
  files.map { |file| calc_word_counts(file) }
end

7. 展開してスッキリ書く

*を使うことで配列やハッシュを展開してスッキリ書くことができます。

配列の場合下記のようになります。

展開前のコード

def fetch_details(files)
  details = files.map { |file| calc_word_counts(file) }

  # 合計を追加している
  details << calc_total_details

  details
end

展開したコード

def fetch_details(files)
  details = files.map { |file| calc_word_counts(file) }
  [*details, calc_total_details]
end

上記2つは同じコードですが、展開した方がスッキリして読みやすいです。
詳しくは下記の記事も参考にしてください↓伊藤さん、毎回ありがとうございます笑

ちなみに、ハッシュの場合は**を使ってキーワード引数として渡すことができます。

例

vegetable = { carrot: 2, onion: 1, cucumber: 3 }
calc_total_amount(**vegetable)

使う場面は少ないかもですが、覚えておくといいかもです。

8. メソッド名に関するその他慣習

メソッドの命名に関するその他の慣習です。

• create_〇〇 → DBにデータを保存する
• set_〇〇 → インスタンス変数に代入する(コントローラーでよくみる)

例

# タスクをDBに保存する
def create_task(params)
  task = Task.new(params)
  task.save
end

# コントローラー内で
before_action :set_goal

def set_goal
  @goal = Goal.find(params[:id)
end

基本setは、アクセサなどに使うため命名に使うべきではないことに注意です。
なので、コントローラーなどでよくみる慣習なのだと思います。

また、_infoや_dataなどは使わない方ベターです。
なぜなら、もっと具体的な命名をした方がコードがわかりやすいからです。

Badな例

user_data = User.find(params[:id])

fruit_array = ['バナナ', 'パイナップル', 'マンゴー']

Goodな例

user = User.find(params[:id])

tropical_fruits = ['バナナ', 'パイナップル', 'マンゴー']

Goodな方がわかりやすい命名になりましたね。

9. FatControllerを解消する

コントローラーに複雑な処理を書くべきではないです。
なぜなら、コントローラーは他の処理の基点やHTTPレスポンスを決めるところだからです。

なので、複雑なロジックがコントローラーに書かれている場合はモデルに移していきましょう。

Badな例

# reports_controller.rb
class ReportsController < ApplicationController
  def create
    @report = current_user.reports.new(report_params)

    # createアクション内で色々している
    report_content = @report.content
    report_ids = report_content.scan(%r{http://localhost:3000/reports/(\d+)}).flatten
    mentioned_reports = Report.where(id: report_ids)

    ActiveRecord::Base.transaction do
      @report.save!
      mentioned_reports.each do |mentioned_report|
        MentionReport.create!(mentioning: @report, mentioned: mentioned_report)
      end
    end

    ...
  end

Goodな例

# reports_controller.rb
class ReportsController < ApplicationController
  def create
    # ロジックをモデルに移行してスッキリした
    @report = current_user.reports.new(report_params)
    @report.save_with_mentions!(report_params)

    ...
  end

# models/report.rb
class Report < ApplicationRecord
  def save_with_mentions!(params)
    ActiveRecord::Base.transaction do
      assign_attributes(params)
      save!
      mentionings.each(&:destroy!)

      mentioned_reports_in_content.each do |mentioned_report|
        MentionReport.create!(mentioning: self, mentioned: mentioned_report)
      end
    end
  end

  def mentioned_reports_in_content
    report_ids = content.scan(%r{http://localhost:3000/reports/(\d+)}).flatten
    Report.where(id: report_ids)
  end
...

createアクションがスッキリしましたね。
ただ、薄いコントローラーにするのってやっぱり難しいです...

下記の記事も参考になります！

10. FatModelを解消する

コントローラーを薄くした後の過程で、FatModelも避けるべきです。

FatModelを解消する方法として

1. Helperやgemを使って対処する
2. app/models/内にクラスを作成する(またはサービスクラスに分割する)

という感じ。
ただ、2以降は正しいオブジェクト指向の理解がないと大変なことになる点には注意です。

1. Helperやgemを使って対処する

• ビューファイルで使うだけ→Helper
• ビューで使うが、モデルと密接に関わる→Decorator
• APIで使うjson系の設定→Serializers

具体的には、下記のような感じで

Helperに分けるコード

class Comment
  # ビューでしか使っていない
  def link_text
    completed_at? ? '完了しました' : '引き続き頑張る'
  end
end

Helperに分ける例

module CommentsHelper
  def link_text(comment)
    comment.completed_at? ? '完了しました' : '引き続き頑張る'
  end
end

モデルに必要ないものは別の箇所に移行できないか考えてみましょう。

2. app/models/内にクラスを作成する(またはサービスクラスに分割する)

app/models/配下に何も継承しないクラスを作成しましょう。
この時、オブジェクト指向でちゃんと考えてクラスを作るのが肝です。

でなければカオスなコードになって、かえってメンテナンスがしずらいコードになります。
下記の例はざっくりしてますが、イメージは掴めると思います。

別クラスに分ける前

# models/user.rb
class User
  def avatar_url
    # 色々やって、画像をリサイズしている
    avatar.blob.analyze unless avatar.blob.analyzed?

    blob = avatar.blob
    width = blob.metadata['width']
    height = blob.metadata['height']

    avatar_size = fetch_avatar_size(width, height)
    avatar.variant(resize_to_fit: avatar_size[:fit], crop: [*avatar_size[:crop], 120, 120]).processed.url

    ...
  end

  # ユーザーアイコンの長さを取得する
  def fetch_avatar_size(width, height)
    ...
    calculate_avatar_size(width, height, :width)
  end

  # ユーザーアイコンの長さを計算する
  def calculate_avatar_size(width, height, long_side)
    
  end

別クラスに分けた後

# models/user.rb
class User
  def avatar_url
    # モデルがスッキリした
    image_resizer = ImageResizer.new(avatar, resize_side: { width: 120, height: 120 })
    image_resizer.resize
    ...
  end
...

# models/image_resizer.rb
class ImageResizer
  def initialize(attachment, resize_side: { width: 600, height: 600 })
    @attachment = attachment
    @resize_side = resize_side
  end

  def resize

  end

  private

  # 画像の辺を取得する
  def fetch_image_side
    
  end

  # リサイズ後の画像の辺を取得する
  def fetch_resize_size
    ...
    calculate_resize_size(long_side)
  end

  # リサイズ後の画像の辺を計算する
  def calculate_resize_size(long_side)
    
  end
end

ただ、FatModel自体はSkinnyControllerと合わせてRailsの方針です。
最近まで、「どっちがいいの？」と思っていたのですが、プロジェクトの方針に合わせるという結論になりました。

個人的には、FatModelも避けたいと思っています。
上手くクラス設計などすれば、コードはさらに読みやすく堅くなると思うからです。

おわりに

最初はRailsが全然わからなくって、というかフレームワークすら未知の概念で...
という状態から、ちょっとわかるかも？に半年でなってきました。

ただ、僕のコードはベテランさんからすると「違う、そうじゃねぇ！」と言いたくなるコードもまだまだ多いです笑
これからも一歩ずつ技術を磨いていきます。

いいねなどしていただけると励みになります。
それでは！