如何改进这段代码

我正在运行一个在线手提包商店，手提包可以是四种颜色 - 黑色，棕色，橙色和红色。我注意到，黑色手袋早于棕色手袋等等。这意味着人们最喜欢黑色手袋。如何改进这段代码

在网上商店的主页上，我想选择和显示网格布局中的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

如何改进这段代码

回答

相关问题