如何改进这段代码
问题描述:
我正在运行一个在线手提包商店,手提包可以是四种颜色 - 黑色,棕色,橙色和红色。我注意到,黑色手袋早于棕色手袋等等。这意味着人们最喜欢黑色手袋。如何改进这段代码
在网上商店的主页上,我想选择和显示网格布局中的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的枚举器。
答
你应该只在一次做类似查询数据库:
@page_offset = ((params[:page].to_i-1)*10) || 0
Handbag.order("color ASC").limit(10).offset(@page_offset)
幸运的颜色已经发生按字母顺序排列。
答
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
答
你可以试试像这样的递归函数。这按预期工作(运行这会给你{: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
+0
只是为了区分接受的答案,这一个将允许您随时更改订单,轻松更改限制,并且只会根据需要运行尽可能多的查询以达到限制 – Fotios
这对[Code Review.SE](http://s.tk/review)是一个更好的问题。 –