「このリファクタリングは思いつかなかった。。。 😲😲 」
はじめに
こんにちは、まつけんです。渋谷のサウナ道場によくいます。
最近エンジニアになって、1年が経ちました。
実装で、手も足も出ないことが無くなってきて、成長を感じてます。
ですが、「もっと短く・分かりやすくできる」というリファクタリングの観点でレビューをもらうことが割とあります。
その中でも「これは、、、気づかなかった。あざます！」となるリファクタリングのベスト5を紹介します。
この記事が参考になりそうな読者
- RubyのリファクタリングTipsを知りたい方
- エンジニア1年目がレビューで指摘されたリファクタリングを知りたい方
※こちらの記事に出てくるコードに関しては全てRubyです。実務で学んだことなので、出てくるコードは全てフィクションです。(実際のサービスのコードではないです)
【第5位】whereを2回以上使うときはこんな感じにできる
ある条件をwhereを使って、2回絞り込んでた時
先輩「whereを2回使って絞り込みをしている箇所の、.where(条件1).where(条件2) のところは .where(条件1, 条件2) のように書けます。」
僕「and条件のリファクタリングはそういう感じでできるんですね、」
【Before】
whereメソッドを2回使っている状態。
- query.where(name: 'Alice')
- .where(detail: { age: 25 })
【After】
whereメソッドの使用を1回に抑えてます。
+ query.where(name: 'Alice', detail: { age: 25 })
理解の参考になる記事
【第4位】ブロックを使う
each文を使ってたとき
先輩「ブロックの書き方を{}を使った書き方にすれば、より短くできるかもです！」
僕「do～endは、{} を使うことでスッキリ書くことができるんですね！勉強になります！」
【Before】
numbers = [1, 2, 3, 4, 5]
sum = 0
- numbers.each do |num|
- sum += num
- end
【After】
do~end→{}に修正。3行が1行に。
余談ですが、ちょうどrubocopの「メソッドの行数多いねん！（Method has too many lines.）」というエラーに引っかかってたので、助かりました。
numbers = [1, 2, 3, 4, 5]
sum = 0
+ numbers.each { |num| sum += num }
理解の参考になる記事
【第3位】早期リターン
ある変数に値が入っているかどうかで条件分岐が発生する処理のレビュー
先輩「if文の条件を逆にして、return default_response if some_type.nil?とするとネストが深くならずに済みます。早期リターンという手法です」
僕「早期リターン？？」
【Before】
変数some_typeの中身によって、条件分岐させていました
- if some_type.present?
- # 処理
- else
- default_response
- end
【After】
こうすることで、以下のメリットがあります。
- 可読性の向上：階層が深くならず、かつ少ない行数で読みやすい
- パフォーマンスの向上：条件が
falseの場合、早期にリターンすることで、不要な処理をスキップできる
+ return default_response if some_type.nil?
+ # 処理
理解の参考になる記事
▼この記事が早期リターンについて、分かりやすく書かれてあります。
- 早期リターンを使うことで、ネストが深くならずに済むところがはっきりとわかります
【第2位】長い処理はメソッドにできないか
〜〜〜ある数値を超えると条件が切り替わる分岐を作ってた時〜〜〜
先輩「この条件分岐、.min使ったら1行にできると思います」
僕「1行？ それはやるべきですね。やり方ググります。」
【Before】
- if (ruby_start..ruby_end).count > 10
- abc = 10
- elsif (ruby_start..ruby_end).count <= 10
- abc = (ruby_start..ruby_end).count
- end
【After1】
minメソッドを使う例。5行が１行に。
+ abc = [(ruby_start..ruby_end).count, 10].min
【After2】
clampメソッドを使う例。以前書いた記事についたこちらのコメントで初めて知りました。最大・最小値をまとめて制約できるメソッド。（詳細）
+ abc = (ruby_start..ruby_end).count.clamp(..10)
理解の参考になる記事
▼ 以前書いた下記の記事でもminメソッドを用いた長い処理のメソッド化については触れています。
▼ clampメソッドの分かりやすい記事
【第1位】ロケールファイルを使う
映えある第1位です。これだけ覚えて、ブラウザバックしてください。
引数によって返り値の条件分岐が発生するメソッドのレビュー
先輩「ロケールファイルに定義しとくと簡単に取れるのかなとも思いました。ホニャララホニャララ〜」
僕「 
！！！！！ 」
これは圧巻でした。
レビューで指摘されるまで、絶対にこのリファクタリングは思いつきませんでした。
【Before】
status_labelメソッドの中で引数status_codeの中身によって、case文を使って、条件分岐させています
def status_label(status_code)
- case status_code
- when 'typeA'
- 'ラベルA'
- when 'typeB'
- 'ラベルB'
- when 'typeC'
- 'ラベルC'
- end
end
【After】
locale.yml（ロケールファイル）を使うことで、表示の切り替えを柔軟にできる実装になりました。
locale.yml（ロケールファイル）に定義しておくことで、今後「ラベルA」などの名前を変更する必要が生じた場合でもlocale.ymlの該当部分だけを編集すれば良いです。
def status_label(status_code)
+ Detail.human_attribute_name("status_label.#{status_code}") if status_code.present?
end
detail:
status_label:
typeA: ラベルA
typeB: ラベルB
typeC: ラベルC
理解の参考になる記事
▼この記事がロケールファイルについて、分かりやすく書いてありました
終わりに
こうやって記事にして、先輩からいただいたリファクタリングを振り返ることで
- 復習として自分のため(成長）にもなるし、
- 分かりやすい説明は読者のためにもなるし、
- 自分が記事を書いて成長することで周りのチームメンバーに利益を与えるためにもなるし、
三報よし。そんな感じの意識高いことを思っております。
「まず動くコードを書く」という意識が先行したエンジニア1年目だったと思います。
「これ、リファクタリングできるよね？」という目線もエンジニア2年目は意識していきたいなあと思ってたので、今回記事を書きました。
意識高いこと書いてたら、少し疲れたのでこれからサウナに行きます。
▼他にもつよつよエンジニアさんから頂いたガチレビューの記事を書いてます。