何をしたか

if文を多用している箇所を、Decoratorを利用してhashで管理するようにしました。

これにより、View上で 35行 あったコードを 16行 まで減らすことができました。
また、同じ文言が複数箇所に散らばっていたため、一箇所で管理するようにし、改修しやすくしました。

どんなメリットがあるか

Viewファイルの可読性の向上
文言修正の依頼があった場合、変更箇所が一箇所で済むようになった

具体的な内容

Userは3つの状態を持つ2種類のステータスをenumで管理していました。

class User
  enum :hoge_status, { a: 0, b: 1, c: 2 }, prefix: true
  enum :fuga_status, { a: 0, b: 1, c: 2 }, prefix: true
end

Viewではそれぞれのステータスで表示する内容を切り替えたかったようです。
また、ステータスがCの時のみCになった時刻が別カラムで保存されるようになっており、
その時間を表示したいという要望だったようです。

h3 ー ステータスhogeについて ー
- if @user.hoge_status_a?
  p ステータスhogeはAです。
- if @user.hoge_status_b?
  p ステータスhogeはBです。
- if @user.hoge_status_c?
  p ステータスhogeはCです。
  p = l(@user.hoge_status_at)

- unless @user.fuga_status_a?
  h3 ー ステータスfugaについて ー
  - if @user.fuga_status_b?
    p ステータスfugaはBです。
  - if @user.fuga_status_c?
    p ステータスfugaはCです。
    p = l(@user.fuga_status_at)

また、上記はユーザー側で、管理者側のユーザー詳細のページでも、このステータスを確認することができます。
そして、管理者側では下記のように、ステータスfugaがaであっても、aであることが明示的に表示されるようになっています。

h3 ー ステータスfugaについて ー
- if @user.fuga_status_a?
  p ステータスfugaはAです。
- if @user.fuga_status_b?
  p ステータスfugaはBです。
- if @user.fuga_status_c?
  p ステータスfugaはCです。
  p = l(@user.fuga_status_at)

実際は上記のようにシンプルではないですが、
今回は条件分岐にフォーカスするため割愛しています。

これらを踏まえてやりたかったこと

このViewの文言はユーザー側だけでなく管理者側でも利用しているため、使いまわしたい
条件分岐の可読性を上げたい

どのようにしたか

初めはif文をcase文に書き換えviewで切り替えていましたが、
コードを見ていくと、文言だけ切り替えればいいことに気づき、
Docoratorに切り出すことにしました。
また、Decoratorの中でもcase文を使用していたのですが、
途中からHashへ変更しました。

ifからcaseへ

case文で記述することで@user.hoge_status_〇〇を複数回記述する必要がありません。
また、case文によって ある1つのステータスの値 によって条件分岐していることが、わかりやすくなりました。

今回はhoge_statusという短い属性名だったので問題ないのですが、
実際のコードでは、hoge_hogehoge_fugafuga_hoge_statusのようなやたらと長い属性名であったため、初めのif文を見た際に「if文ごと全て違うステータスを判定しているのか？」と思い、ステータスの確認から始めていました。
調べてみると、全て同じステータスだったため、コードリーディングに無駄な時間がかかってしまっていました。

Before

h3 ー ステータスhogeについて ー
- if @user.hoge_status_a?
  p ステータスhogeはAです。
- if @user.hoge_status_b?
  p ステータスhogeはBです。
- if @user.hoge_status_c?
  p ステータスhogeはCです。
  p = l(@user.hoge_status_at)

h3 ー ステータスfugaについて ー
- if @user.fuga_status_a?
  p ステータスfugaはAです。
- if @user.fuga_status_b?
  p ステータスfugaはBです。
- if @user.fuga_status_c?
  p ステータスfugaはCです。
  p = l(@user.fuga_status_at)

After

h3 ー ステータスhogeについて ー
- case @user.hoge_status
- when 'a'
  p ステータスhogeはAです。
- when 'b'
  p ステータスhogeはBです。
- when 'c'
  p ステータスhogeはCです。
  p = l(@user.hoge_status_at)

h3 ー ステータスfugaについて ー
- case @user.fuga_status
- when 'a'
  p ステータスfugaはAです。
- when 'b'
  p ステータスfugaはBです。
- when 'c'
  p ステータスfugaはCです。
  p = l(@user.fuga_status_at)

viewからDecoratorへ

段々とコードの全貌が見えてきたため、文言のみ変えれば良いということに気がつきました。

ロジックがviewから少なくなったため見通しが良くなり、
viewではレイアウトなどの情報に集中しやすくなりました。

Before

h3 ー ステータスhogeについて ー
- case @user.hoge_status
- when 'a'
  p ステータスhogeはAです。
- when 'b'
  p ステータスhogeはBです。
- when 'c'
  p ステータスhogeはCです。
  p = l(@user.hoge_status_at)

h3 ー ステータスfugaについて ー
- case @user.fuga_status
- when 'a'
  p ステータスfugaはAです。
- when 'b'
  p ステータスfugaはBです。
- when 'c'
  p ステータスfugaはCです。
  p = l(@user.fuga_status_at)

After

h3 ー ステータスhogeについて ー
p = @user.hoge_status_text
- if @user.hoge_status_c?
  p = l(@user.hoge_status_at)

h3 ー ステータスfugaについて ー
p = @user.fuga_status_text
- if @user.fuga_status_c?
  p = l(@user.fuga_status_at)

user_decorator.rb

def hoge_status_text
  case hoge_status
  when 'a'
    'ステータスhogeはAです。'
  when 'b'
    'ステータスhogeはBです。'
  when 'c'
    'ステータスhogeはCです。'
  end
end

def fuga_status_text
  case fuga_status
  when 'a'
    'ステータスfugaはAです。'
  when 'b'
    'ステータスfugaはBです。'
  when 'c'
    'ステータスfugaはCです。'
  end
end

caseからhashへ

プログラミング中級者になりたい人のためのクリーンコード入門
の「条件分岐」に関する章で、

同じような処理をswitch文で書くと冗長な記述が増えてしまうため、連想配列でまとめられないか検討する。

とあったため、応用してみることにしました。
(また、私の現場のRubocopではcaseをhashにしろと怒られていました。)

Before

user_decorator.rb

def hoge_status_text
  case hoge_status
  when 'a'
    'ステータスhogeはAです。'
  when 'b'
    'ステータスhogeはBです。'
  when 'c'
    'ステータスhogeはCです。'
  end
end

def fuga_status_text
  case fuga_status
  when 'a'
    'ステータスfugaはAです。'
  when 'b'
    'ステータスfugaはBです。'
  when 'c'
    'ステータスfugaはCです。'
  end
end

After

user_decorator.rb

def hoge_status_text
  text = {
    'a' => 'ステータスhogeはAです。',
    'b' => 'ステータスhogeはBです。',
    'c' => 'ステータスhogeはCです。'
  }

  text[hoge_status]
end

def fuga_status_text
  text = {
    'a' => 'ステータスfugaはAです。',
    'b' => 'ステータスfugaはBです。',
    'c' => 'ステータスfugaはCです。'
  }

  text[fuga_status]
end

今後

今回の件を通じて、コードにはさまざまな記述方法ができることを実感し、
そのケースに応じて適宜使い分けることが大切だということを知りました。

参考文献

プログラミング中級者になりたい人のためのクリーンコード入門