Ruby style programming (DRYness)

Rails
1

Hi everybody,

I have a line of the form:

x = hash1.reject{ |key,value| !value}.keys.join(’,’)

That worked fine but then I needed to do the same thing for hash2. So
I had to add another line like so:

y = hash2.reject{ |key,value| !value}.keys.join(’,’)

I’m trying go DRY but I can’t think of an elegant way to do it – I’m
a relative beginner in ruby. A function is an obvious candidate, but
that’s probably my php thinking. Any better way to avoid the
repetition? Some fancy loop?

Thanks!
Sergei

2

On Sep 19, 12:19 am, surge [email protected] wrote:

I’m trying go DRY but I can’t think of an elegant way to do it – I’m
a relative beginner in ruby. A function is an obvious candidate, but
that’s probably my php thinking. Any better way to avoid the
repetition? Some fancy loop?

Thanks!
Sergei

Perhaps extending Hash would help:

lib/hash.rb

class Hash
def get_non_empty_keys # use whatever name is appropriate
self.reject{|k, v| !v}.keys.join(‘,’)
end
end

config/environment.rb

require “lib/hash.rb”

You can now use the get_non_empty_keys function on your hashes:

x = hash1.get_non_empty_keys
y = hash2.get_non_empty_keys

3

Erol, that’s so cool. I like your solution a lot.

What about this one:

I allow users to filter tickets based on the module (I call them
components) they report on. The list of components comes in
as an array. Here’s what I do.

Get a list of components

@components = Component.find :all

This is the hash that will be used in a SQL statement. It will

become clear in a second.
@filter_components = {}

Initialize all components to true (this is the default – we show

tickets for all components)
@components.each { |component| @filter_components[component.id] =
true }

if request.xhr?
@components.each do |component|
# Shouldn’t happen, but if so, empty @filter_components. This
means no tickets will be selected.
if params[:filter_components].empty?
@filter_components = {}
break;
end

if (params[:filter_components]).include?(component.id.to_s)
  @filter_components[component.id] = true
else
  @filter_components[component.id] = false
end

end

Make sure at least one component is in @filter_components so that

we don’t have special cases.
@filter_components[0] = true if @filter_components.size == 0
end

Now comes the line about which I was asking in my previous email.

The result of it is used in an IN() part of a SQL query
components = @filter_components.reject{ |key,value| !
value}.keys.join(’,’)

It works but it’s a lot of code and the problem is I need to do the
same for
statuses! Any way to reduce this and make it reusable for statuses?
BTW, the
reason why I’m not using params[:filter_components] directly in the
SQL query
(e.g. params[:filter_components].join(’,’)) is so that the data from
the web is
not used in a SQL query directly. Make sense?

Thanks!!!

P.S.: I typed this twice! The first version of this was lost. Bummer.

4

On Sep 18, 6:53 pm, surge [email protected] wrote:

Erol, that’s so cool. I like your solution a lot.

What about this one:

I allow users to filter tickets based on the module (I call them
components) they report on. The list of components comes in
as an array. Here’s what I do.

Why aren’t you just doing Ticket.find :all, :conditions =>
{:component_id => params[:component_ids}} ?

Fred

5

Unless you wanted to add that method to the Hash class, you could
easily do:

[hash1, hash2].collect{|hash| hash.reject{ |key,value| !
value}.keys.join(’,’)}

Which would give you an array of the new hashes. If you wanted to
assign these values to x and y:

x,y = [hash1, hash2].collect{|hash| hash.reject{ |key,value| !
value}.keys.join(’,’)}

6

I thought it would be perfect too, but there’s a slight problem. The
limitation is that that form assumes everything as AND’ed. This is
what I came up with yesterday:

@tickets = Ticket.find :all, :conditions => [
“((assignee_id IS NULL AND reporter_id = :reporter_id) OR
assignee_id = :assignee_id)
AND tickets.status_id IN(:statuses) AND
tickets.component_id IN(:components)”,
{
:assignee_id => user.id,
:reporter_id => user.id,
:statuses => @filter_statuses,
:components => @filter_components
}
]

See that OR? I think because of it I can’t use the suggested by
Frederick form. And with the form presented here, I couldn’t say
tickets.component_id = :components. Rails didn’t transform the equal
sign into an “IN” although it did translate the array of component ids
into a comma separated list. But it works nonetheless if I put IN
inside the string. Then, if components are empty, it correctly
replaces the variable with a NULL. And I think everything is properly
escaped as well.

7

On Sep 19, 2:25 am, Frederick C. [email protected]
wrote:

Why aren’t you just doing Ticket.find :all, :conditions =>
{:component_id => params[:component_ids}} ?

I’d go with this one from Fred. No need to worry about using the
params[:component_ids] directly in the finder, the input is being
sanitized by Rails before being used in the SQL, much like :conditions
=> [‘field = ?’, value] is. It generates an IN() condition if
params[:component_ids] is an array or an IS NULL condition if
params[:component_ids] is nil.