From: Giles Bowkett <gilesb@...>
Date: 2006-09-27T01:50:54+09:00
Subject: Re: ruby wizards, help me beautify skanky code

OK -- here's what I have at the moment.

controller:

  def drugs
    Interaction.find(:all, :include =>
"drug").group_by(&:involvement_type).each do |key, value|
      instance_variable_set( "@#{key.downcase.pluralize}",
                             value.map { |i|i.drug.name}.uniq.sort )
unless value.nil?
    end
    render(:partial => "drugs")
  end

view:

<% for thing in %w{ Inducer Inhibitor Substrate } %>
  <% things = eval("@#{thing.downcase.pluralize}") %>
  <% unless things.nil? %>
    <h2><%= thing %>s</h2>
      <ul><% for name in things %>
      <li><%= name %></li>
      <% end %></ul>
  <% end %>
<% end %>

I chose uniq over to_set because I like the way word ends with the
letter q. (not entirely scientific, I must admit.) I tested both just
to see if they would in fact return the same result and in fact they
returned the same stuff but in a different order, so I added sort to
alphabetize the array contents. the unless value.nil? proved necessary
since some values were in fact returning nil. that could be a bug in
the code, or it could be something I overlooked about the data. not
sure.

the view doesn't look half as pretty as the controller but I think it
also benefits from its brevity, although to be honest I'm not entirely
certain of that. the original version was clunky but obvious; the new
version won't present any problem to a maintenance programmer, as long
as the maintenance programmer is me, and has had some caffeine. the
other guy most likely to work on this code besides myself is a
marketing guy who figured out just enough Rails to be dangerous; there
is a risk in handing him templates which contain eval() statements. I
used the things= assignment at the top to lessen the potential pain;
sometimes adding unnecessary steps is actually a kindness to whoever
maintains the code.

it also has the benefit of reducing the number of eval()s from two to
one. I'm hesitant about eval() statements in general, it seems like
the type of programming which evokes the harshest cries of "Perlish!!"
from the Python community. I think the Python community needs to
lighten up a bit, but there's no denying it looks a bit more hacky
than the original did, even tho the original was unimaginative. very
much undecided about eval(). that's pretty much how references work in
Perl, but Perl references are notorious for spreading confusion
everywhere they go. on the other hand, they're also wonderfully
flexible.

anyway, cheers for the help!

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