Как я могу улучшить этот код Rails?

Question

Я пишу небольшую браузерную игру как проект для изучения RoR, и я довольно новичок в этом.

Это небольшой метод, который регулярно вызывается cronjob.

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

def self.restock_energy_potions
  market = find_or_create_market

  potions = EnergyPotion.find_all_by_user_id(market.id)

  while (potions.size < 5)
    potion = EnergyPotion.new(:user_id => market.id)
    potion.save
    potions = EnergyPotion.find_all_by_user_id(market.id)
  end    
end

Answer 1

Я не уверен, что понимаю ваш вопрос. Вы ищете что-то подобное?

def self.restock_energy_potions
  market = find_or_create_market   
  potions = EnergyPotion.find_all_by_user_id(market.id)
  (potions.size...5).each {EnergyPotion.new(:user_id => market.id).save }
  end    
end

Обратите внимание на тройные точки в диапазоне; Вы не хотите создавать зелье, если уже есть 5.

Кроме того, если ваши зелья были связаны (например, с помощью has_many), вы могли бы создать их через свойство market.potions (я предполагаю здесь, об отношениях между пользователями и рынками - детали зависят от того, как ваши модели настроить) и сохранить их все сразу. Я не думаю, что экономия базы данных была бы значительной, хотя.

Answer 2

а) используйте ассоциации:

class Market < AR::Base
  # * note that if you are not dealing with a legacy schema, you should
  #   rename user_id to market_id and remove the foreigh_key assignment.
  # * dependent => :destroy is important or you'll have orphaned records
  #   in your database if you ever decide to delete some market
  has_many :energy_potions, :foreign_key => :user_id, :dependent => :destroy
end

class EnergyPotion < AR::Base
  belongs_to :market, :foreign_key => :user_id
end

b) нет необходимости перезагружать ассоциацию после добавления каждого. также перенести функциональность в модель:

find_or_create_market.restock

class Market
  def restock
    # * note 4, not 5 here. it starts with 0
    (market.energy_potions.size..4).each {market.energy_potions.create!}
  end
end

в) также создать примечание! и не создавать. Вы должны обнаружить ошибки. обработка ошибок зависит от приложения. в вашем случае, так как вы запускаете его из cron, вы можете сделать несколько вещей * отправить письмо с предупреждением * ловить исключения и регистрировать их (плагин exception_notifier или служба hoptoad) * Печать в stderror и настройка cron для отправки сообщений об ошибках по электронной почте.

 def self.restock_potions
    market = find_or_create
    market.restock
  rescue ActiveRecord::RecordInvalid
    ...
  rescue
    ...
  end

Answer 3

Предполагая, что ваш рынок / пользователь has_many зелий, вы можете сделать это:

def self.restock_energy_potions
  market = find_or_create_market
  (market.potions.size..5).each {market.potions.create(:user_id => market.id)}
end