How can I improve this Rails code?

Question

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

This is a little method that's called regularly by a 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

Answer 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.

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.