From: Giles Bowkett <gilesb@...>
Date: 2006-09-25T18:26:12+09:00
Subject: ruby wizards, help me beautify skanky code

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!

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>

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

in fact that's probably what I'll do -- but what with the whole
metaprogramming thing, I'd like to come up with something cleaner, one
of those Lispy things. any ideas would be massively appreciated. I
suppose actually in the course of writing this e-mail I've figured out
the answer to the question myself, but I'm pretty sure there's a next
step beyond this which would make the code more elegant.

-- 
Giles Bowkett
http://www.gilesgoatboy.org