2010-01-12 36 views
0

我很難決定如何在我的控制器中重構這個方法。這個想法是(在這種情況下)它繪製了過去兩週加入(或創建)的用戶。在我的控制器中重構一個簡單的方法

您可能想知道爲什麼我做了@graph_limit的事情,那是因爲我總是希望結果最好的一天成爲我的條形圖上最高的條形圖(在視圖中,只是通過使用css創建使用css的<div>的高度)。

基本上我要幹它,......你知道只是改進這種方法,儘可能:

# Controller 
def index 

    two_weeks_ago = Date.today - 13.days 

    @users_graphed = User.count(:conditions=>["created_at >= ?", two_weeks_ago], :order => 'DATE(created_at) DESC', :group => ["DATE(created_at)"]) 

    two_weeks_ago.upto(Date.today) do |day| 
    @graph_limit = 100/@users_graphed.values.max.to_f 
    @users_graphed[day.to_s] ||= 0 
    end 

end 

而且我要指出,那你們很可能會撕裂我的代碼撕碎...所以我正在爲結果而支撐。

# View 
<% @users_graphed.sort.reverse.each do |user| %> 
    <li>  
    <% content_tag :div, :style => "height: #{number_with_precision(user[1] * @graph_limit, :precision => 2)}px; ", :class => "stat_bar" do %> 
     <%= content_tag(:span, user[1]) unless user[1] == 0 %> 
    <% end %> 
    </li> 
<% end %> 

歸根結底,什麼我這裏真正的目標是要把它放到我的應用程序控制器,並能以圖表的任何模型通過它的create_at倍。可能類似於tasks.chart_by(2.weeks)。你們會如何將這些分離出來,成爲我可以在整個應用程序中使用的東西?

+0

出於某種原因,我覺得它應該是一個命名的範圍伴隨着一個幫手......我寧願在這種情況下有一個非常瘦的控制器。 – 2010-01-12 05:11:01

回答

3

我同意約瑟夫,你的控制器在這裏做了很多工作,應該在模型中完成。任何時候當你在你的控制器中指定多個find參數時,問問自己是否應該在你的模型中。

你在這裏做了很多迭代,似乎不必要。首先,你不應該在循環內計算@graph_limit。你重新計算了14次,但每次的價值都是一樣的。在循環之外這樣做。

其次,你認爲sort.reverse突出。你已經在你的發現者中排序(:order => 'DATE(created_at) DESC'),然後你在視圖中再次排序,然後倒轉它?您應該向數據庫詢問您想要的最終訂單中的值。然後,使您的零填充代碼工作,您可以反轉它,執行Date.today.downto(two_weeks_ago)而不是upto

我會說你應該真的在SQL中做這些事情,但不幸的是(如你可能已經發現的那樣),MySQL很難在沒有創建日曆表的情況下填充缺失的日子。

0

感謝喬丹,每次你的想法(這是順便說真正偉大的),我創建了一個助手就是喜歡這樣的:

def graph_by_time(klass, time_ago) 
    time_range_start = Date.today - time_ago 

    @elements_graphed = klass.count(:conditions=>["created_at >= ?", time_range_start], :order => 'DATE(created_at) DESC', :group => ["DATE(created_at)"]) 
    @graph_limit = 100/@elements_graphed.values.max.to_f 

    time_range_start.upto(Date.today) do |element| 
    @elements_graphed[element.to_s] ||= 0 
    end 

    return @elements_graphed.sort.reverse 
end 

這裏最大的問題是零填充它沒有記錄的天與他們相關聯,您從upto切換到downto的方法沒有起作用,只返回了導致非零整數的記錄。

相關問題