如何改進這段代碼

我正在運行一個在線手提包商店，手提包可以是四種顏色 - 黑色，棕色，橙色和紅色。我注意到，黑色手袋早於棕色手袋等等。這意味着人們最喜歡黑色手袋。如何改進這段代碼

在網上商店的主頁上，我想選擇和顯示網格佈局中的10袋。所以我首先選擇黑色的包包。如果我的庫存中有10個或更多的黑色袋子，那麼我會停下來，不要去尋找其他顏色的袋子。但是，如果我有5個黑色包包，我會繼續尋找棕色包包。如果我還沒有10袋，那麼加入那些棕色袋子後，我會尋找橙色袋子等等。

下面是我在實施這一方案爲Rails的模型方法的嘗試：

class Handbag < ActiveRecord::Base 
    belongs_to :store 
    attr_accessor :color 
end 

class Store < ActiveRecord::Base 
    has_many :handbags 

    def handags_for_display 
    selected_handbags = [] 
    ["black", "brown", "orange", "red"].each do |color| 
     bags = get_handbags_by_color(color) 
     selected_bags += bags 
     if selected_bags.size >= 10 
     selected_handbags = selected_handbags[0..9] 
     break 
     end 
    end 
    selected_handbags 
    end 

    private 
    def get_handbags_by_color(color) 
    handbags.where("color = ?", color).limit(10) 
    end 
end

雖然這個作品，我很好奇，如果有更好的方式來寫它。特別是，我認爲這個代碼可以轉換爲使用Ruby的枚舉器。

來源

2011-10-21 ardsrk

這對[Code Review.SE]（http://s.tk/review）是一個更好的問題。 –

你應該只在一次做類似查詢數據庫：

@page_offset = ((params[:page].to_i-1)*10) || 0 
Handbag.order("color ASC").limit(10).offset(@page_offset)

幸運的顏色已經發生按字母順序排列。

來源

2011-10-21 12:53:42 Dex

這不會工作，因爲他有超過10黑袋.... – davidb

它已被固定，以適應這種情況。另外，誰投票給我，請解釋原因。 – Dex

@Dex您的解決方案可以正常工作。在使用模型時，我必須習慣於使用SQL而不是Ruby來思考。順便說一句，沒有必要分頁，因爲我從來沒有展示超過10個手袋。 – ardsrk

-1

def handags_for_display 
    handbags = Array.new 
    [{ :color => 'black', :count => 5 }, { :color => 'brown' , :count => 2 }, { :color => 'orange', :count => 1 }, { :color => 'red', :count => 1}].each do |handbag| 
      handbags+=Handbag.where("color = ?", handbag[:color]).limit(handbag[:count]) 
    end 
    handbags 
end

來源

2011-10-21 13:02:07 davidb

你可以試試像這樣的遞歸函數。這按預期工作（運行這會給你{:black => 1, :brown => 8, :orange => 1}），你可以改變get_handbags_by_color來代替Rails。

@bags = { 
    :black => 1, 
    :brown => 8, 
    :orange => 10, 
    :red => 10 
} 

@handbag_order = [:black, :brown, :orange, :red] 
@max_bags = 10 

def get_handbags_by_color(color,limit) 
    num = @bags[color] 
    num > limit ? limit : num 
end 

def handbags_for_display(index = 0, total = 0) 
    color = @handbag_order[index] 
    return {} unless color 
    handbags = {color => get_handbags_by_color(color,@max_bags - total)} 
    total += handbags.values.inject{|sum,x| sum+x} 

    handbags.merge!(handbags_for_display(index+1, total)) unless(total >= @max_bags) 
    handbags 
end 

handbags = handbags_for_display

來源

2011-10-21 13:31:58 Fotios

只是爲了區分接受的答案，這一個將允許您隨時更改訂單，輕鬆更改限制，並且只會根據需要運行儘可能多的查詢以達到限制 – Fotios

如何改進這段代碼

回答

相關問題