still more relentless non-repetition

[Giles Bowkett]

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!)

 

[Devin Mullins]

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]

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

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

 

[dblack]

Hi --

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 | (e-mail address removed)
Author of "Ruby for Rails" [1] | Ruby/Rails training & consultancy [3]
DABlog (DAB's Weblog) [2] | Co-director, Ruby Central, Inc. [4]
[1] http://www.manning.com/black | [3] http://www.rubypowerandlight.com
[2] http://dablog.rubypal.com | [4] http://www.rubycentral.org

 

[ara.t.howard]

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

 

[Giles Bowkett]

Major Ruby-injection problem here. NEVER eval something you get from an

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]

model = self.class.const_get t.capitalize

model.find_by_contents t

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

gracias!

 

[dblack]

Hi --

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 | (e-mail address removed)
Author of "Ruby for Rails" [1] | Ruby/Rails training & consultancy [3]
DABlog (DAB's Weblog) [2] | Co-director, Ruby Central, Inc. [4]
[1] http://www.manning.com/black | [3] http://www.rubypowerandlight.com
[2] http://dablog.rubypal.com | [4] http://www.rubycentral.org

 

[ara.t.howard]

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.

just putting my abhorrence of container methods defined on non-containers on
public display ;-)

-a

 

[Wilson Bilkovich]

**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]

wow that's handy:

thing.constantize.find_by_contents(term)

boom! done.

 

[Giles Bowkett]

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.

 

[Max Muermann]

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.

Try:

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

and

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

Cheers,
Max

 

[David Vallner]

--------------enigAC890B69156F3593D79D893C
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

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

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


--------------enigAC890B69156F3593D79D893C
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (MingW32)

iD8DBQFFSmEKy6MhrS8astoRAkc9AJ4gBW1zs+Xq+4wlYnzcWRB3rl6KKwCbBMMo
ciD7d8P4o4hvVLcL53il4p8=
=4LN/
-----END PGP SIGNATURE-----

--------------enigAC890B69156F3593D79D893C--