From: Daniel N <has.sox@...>
Date: 2006-09-25T21:24:12+09:00
Subject: Re: ruby wizards, help me beautify skanky code

------=_Part_24277_24758408.1159187048270
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

On 9/25/06, Giles Bowkett <gilesb@gmail.com> 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!


I'm assuming that you're using rails here.  Maybe not...  I'm trying to get
everything with one query on the db but I haven't tested it so it may not
work very well.

@interactions = Interaction.find(:all,
                                           :conditions => ["involvement_type
in (?)", %w(Inducer Inhibitor Substrate)],
                                           :include =>
:drugs).group_by(&:involvement_type)


@interactions.keys.each do |key|
  instance_variable_set( "@#{key}", interactions[key].map { |i|
i.drug.name}.to_set )
end


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.
>
> the view file for this process is equally lame:
>
> <h2>Inducers</h2>
> <ul><% for inducer in @inducers %>
> <li><%= inducer %></li>
> <% end %></ul>
>
> <h2>Inhibitors</h2>
> <ul><% for inhibitor in @inhibitors %>
> <li><%= inhibitor %></li>
> <% end %></ul>
>
> <h2>Substrates</h2>
> <ul><% for substrate in @substrates %>
> <li><%= substrate %></li>
> <% end %></ul>


Assuming the above

<% @interactions.keys.each do |key| -%>
  <h2><%= key.titleize -%></h2>
    <ul><% instance_variable_get( "@#{key}" ).each do |drug_name| -%>
     <li><%= drug_name -%></li>
    <% end -%>
    </ul>
<% end -%>

Hope that works for you...

------=_Part_24277_24758408.1159187048270--