From: "Zed A. Shaw" Date: 2006-09-25T19:08:22+09:00 Subject: Re: ruby wizards, help me beautify skanky code On Mon, 25 Sep 2006 18:26:12 +0900 "Giles Bowkett" wrote: > here it is: > > @inducers = [] > @inhibitors = [] > @substrates = [] > > Interaction.find_all_by_involvement_type("Inducer").each do |interaction| > @inhibitors.push(Drug.find(interaction.drug_id).name) > end > Interaction.find_all_by_involvement_type("Inhibitor").each do |interaction| > @inhibitors.push(Drug.find(interaction.drug_id).name) > end > Interaction.find_all_by_involvement_type("Substrate").each do |interaction| > @substrates.push(Drug.find(interaction.drug_id).name) > end > > @inducers.uniq! > @inhibitors.uniq! > @substrates.uniq! require 'set' inv_types = ["Inducer", "Inhibitor", "Substrate"] @results = {} inv_types.each do |inv| m = Interaction.find_all_by_involvement_type(inv).map { |inter| Drug.find(inter.drug_id).name) } @results[inv] = Set.new(m) end But I didn't run this, don't know how Drug objects interact with Set, and I'm sure it could be made more readable or faster (but not both). Also you could do a SQL statement or fancier find that'll make this much more efficient. Advantage of this is when you get new involvement types you just update the inv list, and you could probably even do away with that and place those in a table instead. Your views can also change to just iterate over the contents of @results and become general as well. -- Zed A. Shaw, MUDCRAP-CE Master Black Belt Sifu http://www.zedshaw.com/ http://mongrel.rubyforge.org/ http://www.lingr.com/room/3yXhqKbfPy8 -- Come get help.