From: dblack@...
Date: 2006-09-25T19:14:53+09:00
Subject: Re: ruby wizards, help me beautify skanky code

Hi --

On Mon, 25 Sep 2006, 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)

I assume you mean @interactions.push here.

>   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!
>
> obviously there's quite a bit of duplication in this. I am utterly
> certain that with currying or something similar, this code could be
> much, much briefer, much more DRY.
>
> the "uniq!" calls are nonoptimal, but I didn't see any obvious way to
> exclude duplicates in the process of building the arrays without using
> multiple ActiveRecord find() calls, and since those turn into SQL
> queries, it seemed wasteful.

Here's an untested version that might be improved upon but nonetheless
might give you some ideas:

   %w{ inducer inhibitor substrate }.each do |thing|
     condition =
     instance_variable_set("@#{thing}s",
       Interaction.find(:all,
                        :conditions => "involvement_type = #{thing.upcase},
                        :include    => "drug").map {|i| i.drug.name}.uniq
   end

I've taken the liberty of changing Drug.find(interaction.drug_id).name
to i.drug, on the theory that if interactions have a drug_id field,
then they belong_to :drug, and therefore should have a drug method.


David

-- 
                   David A. Black | dblack@wobblini.net
Author of "Ruby for Rails"   [1] | Ruby/Rails training & consultancy [3]
DABlog (DAB's Weblog)        [2] | Co-director, Ruby Central, Inc.   [4]
[1] http://www.manning.com/black | [3] http://www.rubypowerandlight.com
[2] http://dablog.rubypal.com    | [4] http://www.rubycentral.org