Still more relentless non-repetition

ruby-talk
1

ok, I have this Rails code which I want to make more Rubyish.

controller:

  def tracks
    @term = params[:term] || ''
    if @term.blank?
      @tracks = []
    else
      @tracks = Track.find_by_contents @term
    end
  end

view:

<% for word in %w{ tracks beats users profiles } %>

  <% unless controller.action_name == word %>
    <%= link_to "search similar #{word}", :action => word, :term => @term %>
    <br/>
  <% end %>

<% end %>

now this is using Ferret, the Ruby port of Lucene, via the
acts_as_ferret Rails plugin.

(by the way, Ferret actually runs faster than Lucene, according to the
Ferret site. I don't know how, but that's pretty cool.)

anyway, as you can see, I have one method in the controller and a view
which assumes the existence of three additional methods.

what I want to do is have only one method in the controller, which
does all the searching.

now I could do:

def tracks
  @term = params[:term] || ''
  case params[:thing_to_search_for]
  when "tracks"
    if @term.blank?
      @tracks = []
    else
      @tracks = Track.find_by_contents @term
    end
  when "beats"
    if @term.blank?
      @beats = []
    else
      @beats = Beat.find_by_contents @term
    end
  when "profiles"
    if @term.blank?
      @profiles = []
    else
      @profiles = Profile.find_by_contents @term
    end
  when "users"
    if @term.blank?
      @users = []
    else
      @users = User.find_by_contents @term
    end
  end
end

however, this is what Rails users call "wet" and everybody else calls "stupid."

what I want to do is a properly elegant thingy. here's roughly what I envision:

  def search
    @term = params[:term] || ''
    instance_variable_set("@#{params[:thing_to_search_for]}", [])
    unless @term.blank?
      eval("@#{params[:thing_to_search_for]}") =
(eval(params[:thing_to_search_for].capitalize)).find_by_contents @term
    end
  end

I think I should probably just start coding in Lisp and save everyone
the irritation, but in the meantime, how does that look, in terms of a
tree to bark up? that is to say, does it look like the right tree, or
the wrong tree?

if you can puzzle it through, I think what I have here is an elegant
algorithm expressed in very-much-other-than-elegant code. (but I could
be wrong!)

···

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

2

I'm too lazy ATM to read the whole thing and make a design recommendation, but Danger, Will Robinson!

     eval("@#{params[:thing_to_search_for]}") =
(eval(params[:thing_to_search_for].capitalize)).find_by_contents @term

Major Ruby-injection problem here. NEVER eval something you get from an untrusted user. Use, instead, instance_variable_get and Object.const_get.

Devin

3

ok, I have this Rails code which I want to make more Rubyish.

controller:

def tracks
  @term = params[:term] || ''
  if @term.blank?
    @tracks =
  else
    @tracks = Track.find_by_contents @term
  end
end

do you really want to search twice here if tracks is called twice?

i'm guessing you'd want

   def tracks
     return @tracks unless defined? @tracks
     t = params[:term]
     @tracks =
       if t.nil? or t.blank?
         
       else
         Track.find_by_contents t
       end
   end

def tracks
@term = params[:term] || ''
case params[:thing_to_search_for]
when "tracks"
  if @term.blank?
    @tracks =
  else
    @tracks = Track.find_by_contents @term
  end
when "beats"
  if @term.blank?
    @beats =
  else
    @beats = Beat.find_by_contents @term
  end
when "profiles"
  if @term.blank?
    @profiles =
  else
    @profiles = Profile.find_by_contents @term
  end
when "users"
  if @term.blank?
    @users =
  else
    @users = User.find_by_contents @term
  end
end

   def tracks
     return @tracks unless defined? @tracks
     t = params[:term]
     @tracks =
       if t.nil? or t.blank?
         
       else
         model = self.class.const_get t.capitalize
         model.find_by_contents t
       end
   end

__never__ use eval on user data in webaps.

regards.

-a

···

On Wed, 1 Nov 2006, Giles Bowkett wrote:
--
my religion is very simple. my religion is kindness. -- the dalai lama

4

Giles Bowkett wrote:

(by the way, Ferret actually runs faster than Lucene, according to the
Ferret site. I don't know how, but that's pretty cool.)

A) All benchmarks lie, -especially- ones people use to pimp their own
products.

I'm not claiming this is the cause though, because:
B) Ferret uses a C extension, and the benchmark is probably that
version, not the pure Ruby one; I imagine advanced fulltext search
algorithms do quite a fair share of numbercrunching, where C would win
out. Of course, with Ruby it's entirely possible marshalling of results
between C and Ruby would kill the pure performance gain (speaking
generally, not of specifically Ferret anymore), but it's possible to
skew a benchmark so that doesn't show - a large dataset and a small
result set for an extension that instead tends to be used frequently for
larger result sets. A full processor usage profile on datasets and
result sets of varying sizes would tell books above a simple benchmark.

David Vallner

5

ah yeah, that's a good point, SQL injection attacks.

···

On 10/31/06, Devin Mullins <twifkak@comcast.net> wrote:

I'm too lazy ATM to read the whole thing and make a design
recommendation, but Danger, Will Robinson!
> eval("@#{params[:thing_to_search_for]}") =
> (eval(params[:thing_to_search_for].capitalize)).find_by_contents @term
Major Ruby-injection problem here. NEVER eval something you get from an
untrusted user. Use, instead, instance_variable_get and Object.const_get.

Devin

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

6

         model = self.class.const_get t.capitalize
         model.find_by_contents t

**that** looks like the way to do it.

gracias!

···

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

7

Hi --

···

On Wed, 1 Nov 2006, ara.t.howard@noaa.gov wrote:

On Wed, 1 Nov 2006, Giles Bowkett wrote:

ok, I have this Rails code which I want to make more Rubyish.

controller:

def tracks
  @term = params[:term] || ''
  if @term.blank?
    @tracks =
  else
    @tracks = Track.find_by_contents @term
  end
end

do you really want to search twice here if tracks is called twice?

i'm guessing you'd want

def tracks
   return @tracks unless defined? @tracks
   t = params[:term]
   @tracks =
     if t.nil? or t.blank?

nil is considered blank by blank?, so you only need the one test.

David

--
                   David A. Black | dblack@wobblini.net
Author of "Ruby for Rails" [1] | Ruby/Rails training & consultancy [3]
DABlog (DAB's Weblog) [2] | Co-director, Ruby Central, Inc. [4]
[1] Ruby for Rails | [3] http://www.rubypowerandlight.com
[2] http://dablog.rubypal.com | [4] http://www.rubycentral.org

8

Hi --

···

On Wed, 1 Nov 2006, Giles Bowkett wrote:

On 10/31/06, Devin Mullins <twifkak@comcast.net> wrote:

I'm too lazy ATM to read the whole thing and make a design
recommendation, but Danger, Will Robinson!
> eval("@#{params[:thing_to_search_for]}") =
> (eval(params[:thing_to_search_for].capitalize)).find_by_contents @term
Major Ruby-injection problem here. NEVER eval something you get from an
untrusted user. Use, instead, instance_variable_get and Object.const_get.

ah yeah, that's a good point, SQL injection attacks.

It's not so much SQL injection as eval injection. Imagine if
params[:thing_to_search_for] is "a=1; system('rm -rf /*')" or
something. You'd be eval'ing the string:

   @a=1; system('rm -rf /*')

David

--
                   David A. Black | dblack@wobblini.net
Author of "Ruby for Rails" [1] | Ruby/Rails training & consultancy [3]
DABlog (DAB's Weblog) [2] | Co-director, Ruby Central, Inc. [4]
[1] Ruby for Rails | [3] http://www.rubypowerandlight.com
[2] http://dablog.rubypal.com | [4] http://www.rubycentral.org

9

just putting my abhorrence of container methods defined on non-containers on
public display :wink:

-a

···

On Wed, 1 Nov 2006 dblack@wobblini.net wrote:

i'm guessing you'd want

def tracks
   return @tracks unless defined? @tracks
   t = params[:term]
   @tracks =
     if t.nil? or t.blank?

nil is considered blank by blank?, so you only need the one test.

--
my religion is very simple. my religion is kindness. -- the dalai lama

10

ActiveSupport has a helper for this already.

model = t.constantize
model.find_by_contents(t)

···

On 10/31/06, Giles Bowkett <gilesb@gmail.com> wrote:

> model = self.class.const_get t.capitalize
> model.find_by_contents t

**that** looks like the way to do it.

gracias!

11

>> Major Ruby-injection problem here. NEVER eval something you get from an
>> untrusted user. Use, instead, instance_variable_get and Object.const_get.
>
> ah yeah, that's a good point, SQL injection attacks.

It's not so much SQL injection as eval injection. Imagine if
params[:thing_to_search_for] is "a=1; system('rm -rf /*')" or
something. You'd be eval'ing the string:

   @a=1; system('rm -rf /*')

true, that would also be very bad.

···

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

12

wow that's handy:

thing.constantize.find_by_contents(term)

boom! done.

···

On 10/31/06, Wilson Bilkovich <wilsonb@gmail.com> wrote:

On 10/31/06, Giles Bowkett <gilesb@gmail.com> wrote:
> > model = self.class.const_get t.capitalize
> > model.find_by_contents t
>
> **that** looks like the way to do it.
>
> gracias!
>

ActiveSupport has a helper for this already.

model = t.constantize
model.find_by_contents(t)

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

13

OK. here's what I have.

  def index
    @term = params[:term] || ''
    @category = params[:category] || ''
    if not %w{ Track Beat User Profile }.include? @category
      redirect_to "/"
    else
      instance_variable_set("@#{@category.downcase.pluralize}",
                            (@category.constantize.find_by_contents @term))
    end
  end

index view:

<%= render_partial @category.downcase.pluralize %>

<%= render_partial "options" %>

options partial:

<% for word in %w{ tracks beats users profiles } %>

  <% if eval("@#{word}").nil? %>
    <%= link_to "search similar #{word}",
                :action => "index",
                :term => @term,
                :category => word.capitalize.singularize %>
    <br/>
  <% end %>

<% end %>

the code in the model-specific partials is design-specific, but it
picks up the instance vars, which is all that matters in this context.

obviously I am using eval and instance_variable_set in ways I'm not
supposed to, but in both cases they're filtered in such a way as to
prevent harm.

actually, if that still seems risky to anybody, I would like to know.
I think it's probably OK but I could easily be guilty of
overconfidence.

I don't like all the upcasing and downcasing. it's not as elegant as
it could be.

what I do like about it is that it generates a lot of HTML without
very much Ruby at all. that's pretty good bang for the buck. in one
short method and two quick partials, you have all the code necessary
to generate four different types of model-specific search pages, plus
links for searching different models with the same query.

obviously the array of words could be an instance var, and could be
aribtrarily large, but only if I fixed the upcasing and downcasing.

···

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

14

I don't like all the upcasing and downcasing. it's not as elegant as
it could be.

what I do like about it is that it generates a lot of HTML without
very much Ruby at all. that's pretty good bang for the buck. in one
short method and two quick partials, you have all the code necessary
to generate four different types of model-specific search pages, plus
links for searching different models with the same query.

obviously the array of words could be an instance var, and could be
aribtrarily large, but only if I fixed the upcasing and downcasing.

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

Try:

"Track".tableize => 'tracks'
'tracks'.classify => Track

and

"MyCategory".tableize => "my_categories"
"my_categories".classify => "MyCategory"

Cheers,
Max