Как улучшить этот кусок кода - PullRequest
Новая
       8

Как улучшить этот кусок кода

1 голос
/ 21 октября 2011

Я управляю интернет-магазином сумок, где сумки могут быть четырех цветов - черный, коричневый, оранжевый и красный.Я заметил, что черные сумки продаются раньше, чем коричневые сумки и так далее.Это означает, что людям больше всего нравятся черные сумки.

На главной странице интернет-магазина я хочу выбрать и отобразить 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.

Ответы [ 3 ]

0 голосов
/ 21 октября 2011 
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
0 голосов
/ 21 октября 2011

Вы можете попробовать рекурсивную функцию, подобную этой. Это работает, как и ожидалось (выполнение этого даст вам {: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 голосов
/ 21 октября 2011

Вы должны сразу запросить базу данных, выполнив что-то вроде: 

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

К счастью, цвета уже расположены в алфавитном порядке.

...