From: Jan Svitok <jan.svitok@...>
Date: 2006-09-25T18:49:18+09:00
Subject: Re: ruby wizards, help me beautify skanky code

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()
typo!!! @inducers
>     end

you can write this as

       @inducers +=
Interaction.find_all_by_involvement_type("Inducer").map do
|interaction|
            Drug.find(interaction.drug_id).name
       end

If you don't mind the order, you can get rid of uniq! by using Set.new
instead of [].

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

You can make this a method in the helper, or use sub-template.

> again, loads of needless repetition. I'm utterly certain that both
> these code samples could be much, much more elegant, but other than a
> few vague ideas about currying I'm really at a loss to say exactly
> how. an inelegant view I can cope with, but the sheer mind-numbing
> repetition of the controller code is utterly mortifying, more so when
> I know with absolute certainty that Ruby has some way of streamlining
> that process immensely, I just don't know specifically what it
> actually is.
>
> I could just turn it into a private method, I suppose, something like
>
>   @inducers = uniq_list("Inducer")
>   @inhibitors = uniq_list("Inhibitor")
>   @substrates = uniq_list("Substrate")
>
>   private
>   def uniq_list(search_term)
>     items = []
>     Interaction.find_all_by_involvement_type(search_term).each do |interaction|
>       items.push(Drug.find(interaction.drug_id).name)
>     end
>     items.uniq! || []
>   end

same here:
  def uniq_list(search_term)
    Interaction.find_all_by_involvement_type(search_term).map do |interaction|
       Drug.find(interaction.drug_id).name
     end.uniq! || []
  end