如何改进此Rails代码？

I'm writing a little browser game as a project to learn RoR with and I'm quite new to it.

我正在编写一个小浏览器游戏作为学习RoR的项目,我对它很陌生。

This is a little method that's called regularly by a cronjob.

这是一个由cronjob定期调用的方法。

I'm guessing there should be some way of adding elements to the potions array and then doing a bulk save at the end, I'm also not liking hitting the db each time in the loop to get the number of items for the market again.

我猜应该有一些方法可以添加元素到魔药数组,然后在最后进行批量保存,我也不喜欢每次在循环中点击数据库以再次获得市场的项目数量。

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

3 个解决方案

#1

I'm not sure I'm understanding your question. Are you looking for something like this?

我不确定我是否理解你的问题。你在找这样的东西吗?

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

Note the triple dots in the range; you don't want to create a potion if there are already 5.

注意范围内的三点;如果已经有5个,你不想创造药水。

Also, if your potions were linked (e.g. by has_many) you could create them through the market.potions property (I'm guessing here, about the relationship between users and markets--details depend on how your models are set up) and save them all at once. I don't think the data base savings would be significant though.

此外,如果您的魔药被链接(例如通过has_many),您可以通过market.potions属性创建它们(我在这里猜测,关于用户和市场之间的关系 - 细节取决于您的模型的设置方式)并保存他们一下子。我不认为数据库的节省会很重要。

#2

Assuming that your market/user has_many potions, you can do this:

假设你的市场/用户有很多药水,你可以这样做:

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

#3

a) use associations:

a)使用协会:

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) no need to reload the association after adding each one. also move the functionality into the model:

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

c) also note create! and not create. you should detect errors. error handling depends on the application. in your case since you run it from cron you can do few things * send email with alert * catch exceptions and log them, (exception_notifier plugin, or hoptoad hosted service) * print to stderror and configuring cron to send errors to some email.

c)还要注意创造!而不是创造。你应该发现错误。错误处理取决于应用程序。在你的情况下,因为你从cron运行它你可以做一些事情*发送带有alert * catch异常并记录它们的电子邮件,(exception_notifier插件或hoptoad托管服务)*打印到stderror并配置cron以向某些电子邮件发送错误。

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

#1