👎
Javaの経験が原因でRubyで陥りがちな悪い実装
概要
RubyやRailsの実装でよくあるミスや、Javaの経験が影響して書いてしまった悪い👎コードについて具体例を紹介します。
やりたい仕様
以下のparameterを持つメソッドがあるとして、「開始年月」+「期間(月)」が現在月までに回答する「期間(月)」を求める実装をしたいです。
- form_ym:開始年月(Time型で月の01日)
- period_month:期間(月単位で整数)
実装の変化
第一段階
最初はコードはまるでjavaのようなコードでした。☺️☺️☺️
def available_period_month(form_ym, period_month)
end_time = form_ym + (period_month - 1).months
if end_time > Time.zone.now.end_of_month
return (Time.zone.now.year * 12 + Time.zone.now.month) - (form_ym.year * 12 + form_ym.month) + 1
end
period_month
end
実装内容を説明すると以下の通りです。
-
end_time = form_ym + (period_month - 1).months
:開始年月から期間(月)分なので、1月分を引いて期間の終了日を計算 -
end_time > Time.zone.now.end_of_month
:期間の終了日が今月末より後の場合 -
return (Time.zone.now.year * 12 + Time.zone.now.month) - (form_ym.year * 12 + form_ym.month) + 1
:月単位の差分を計算 -
period_month
:引数の期間(月)が今月末の後じゃない場合は引数の期間(月)をそのまま返す
実装自体が良いとか悪いとかの問題以前にRubyらしく書き換える前に一旦書いて、これから段々Ruby化?する目的でした。Rubyの有識者ならこの段階はいらないのかな。。。
第二段階
自分的にはrubyっぽく書くために頑張った結果が以下のコードです。javaのStreamのような使い方で、上の例より大分短くてそれでも仕様の理解に悩まない状態になりました。
def available_period_month(form_ym, period_month)
(0...period_month).each do |n|
return n if (form_ym + n.month) > Time.zone.now.end_of_month
end
period_month
end
実装内容は違いますが、仕様は同じです。
-
(0...period_month).each
:ループは0から「period_month - 1」までになる -
return n if (form_ym + n.month) > Time.zone.now.end_of_month
:今月末の後の時点の期間(月)を返し、メソッドが終了 -
period_month
:引数の期間(月)が今月末の後じゃない場合は引数の期間(月)をそのまま返す
最終段階
色んなアドバイスから以下のように変わりました。
def available_period_month(form_ym, period_month)
res = (0...period_month).find { |n| (form_ym + n.month) > Time.zone.now.end_of_month }
res || period_month
end
このコードのfind
ですが、JavaのSpringのJPAやStreamだとFirst等がなければCollection型を返ってくると思いましたが、find
のみで最初に発見されたものを返すようでした。
結論
今回の経験で学んだことは以下の3点です。
- Rubyのマニュアルやリファレンスを読むことの重要性を再確認しました。
- JavaとRubyの思想の違いを意識して、よりRubyらしいコードを書くように努める必要があります。
- Javaのラムダ式やStreamは、Rubyでどのように活用できるかを深く考えるきっかけになりました。
Discussion