Ruby style programming (DRYness)

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

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(',')}

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

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.

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

Fred

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.

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.