Jano Svitok
9/25/2006 9:49:00 AM
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