例えばこんなコード

 1 def csv_data(since_time, till_time)
 2   User.actice_user(since_time, till_time)
 3 end
 4 
 5 def data_count(since_time, till_time)
 6   csv_data(since_time, till_time).size
 7 end
 8 
 9 def run(since_time, till_time)
10   write_csv_file csv_data(since_time, till_time)
11 end

since_time,till_timeが引数として散在している。こんな場合はメジャーな策が主に2つあり、その判別法も割とシンプルだ。 まず考えるのが 重複しているデータがプレーンなデータか?何らか処理後(もしくは処理を行う予定の)のデータか? だ。

  • プレーンなデータの場合
    since_time,till_timeは他のクラスなどで生成されたデータで本クラスのメソッドに渡されているものとし、特に加工せず使い回すデータだとしましょう。そんなプレーンなデータである場合はデータをインスタンス変数として保持すると良いでしょう。具体的には
 1 def initialize(attr={})
 2   @since_time = attr[:since_time]
 3   @till_time  = attr[:till_time]
 4 end
 5 
 6 def csv_data
 7   User.actice_user(@since_time, @till_time)
 8 end
 9 
10 def data_count
11   csv_data.size
12 end
13 
14 def run
15   write_csv_file csv_data
16 end

インスタンス変数を使用した結果、見事に引数の重複はなくなりシンプルになりましたね。 特に加工せず使い回しているデータ というのが重要で、もし途中で処理し値が変化するようなデータだと、インスタンス変数で保持する前に少し踏みとどまって考えた方が良いでしょう。なぜならファイルが長くなった場合(それ自体がアンチパターンなのだが)などにどのタイミングでデータ値が変更されているか把握しづらいので、インスタンス変数の参照時点で、それはもう意図と異なる値かもしれません。

  • 何らか処理後(もしくは処理を行う予定の)のデータの場合
    since_time,till_timeが他のクラスから渡され、本クラスで処理を行った後使い回す場合、もしくは本クラスで処理によって求めたデータであるとしましょう。そのような何らか処理後(もしくは処理を行う予定の)のデータの場合はメソッド化すると良いでしょう。具体的には
 1 def csv_data
 2   User.actice_user(since_time, till_time)
 3 end
 4 
 5 def data_count
 6   csv_data.size
 7 end
 8 
 9 def run
10   write_csv_file csv_data
11 end
12 
13 def since_time
14    Date.today
15 end
16 
17 def till_time
18    Date.today - User.find_by(user_roul: 1, user_active: true).created_at
19 end

メソッドを使用した場合も同様に、重複なくシンプルにすることが出来ました。

1 def since_time
2    @since_time ||= Date.today
3 end
4 
5 def till_time
6    @till_time ||= Date.today - User.find_by(user_roul: 1, user_active: true).created_at
7 end

また場合にもよりますがsince_time,till_timeを取得するのにDBからの取得が伴ったり、重い処理で何度も実行させたくない場合は、この様にメモ化するのが一般的ですね。そもそもメソッド自体をなくしたりみたいな話になると魔術の世界になるので今日はここらでやめておく。




comments powered by Disqus


© 2015 kyuden