4
\$\begingroup\$

Currently a filter parameter of a GET request is used to designate items' categories that will be requested from database and displayed.

The parameter contains comma delimited string - each string id for each category: filter=complete,inwork,cancelled,ycancelled.

I want to get a Ruby object with corresponding attributes that will be true or false dependent on string id presence in filter parameter. Current implementation is as follows:

filter = params[:filter].split(',') rescue []
fopt = OpenStruct.new(Hash[%w(complete inwork cancelled ycancelled).map {|fo| [fo.to_sym, filter.include?(fo)]}])

Could this be rewritten in shorter way?

\$\endgroup\$
3
\$\begingroup\$

Some notes:

  • Don't use a catch-all rescue, bad practice.

  • Don't write lines so long, they are hard to read.

  • Move all logic to models.

I'd write (in some model, not in a controller):

allowed_keys = %w(complete inwork cancelled ycancelled)
params_keys = params.fetch(:filter, "").split(',').map(&:strip).to_set
filter_attributes = allowed_keys.map { |k| [k.to_sym, params_keys.include?(k)] }.to_h
fopt = OpenStruct(filter_attributes)
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Curious, why to_set? To remove duplicates? If so, why not use array.uniq? And doesn't to_set require you to require set?-e.g. require 'set' \$\endgroup\$ – Mohamad Jan 5 '15 at 20:57
  • 1
    \$\begingroup\$ 1) I use to_set not to remove duplicates but because Set#include? is O(1) while Array#include? is O(n). Not really needed with those data sizes, but feels conceptually more fitting. 2) In a bare Ruby script, yes, but Rails already includes set. Doesn't hurt to write it, though. \$\endgroup\$ – tokland Jan 5 '15 at 21:58
3
\$\begingroup\$

Code looks great to me. Here is a Rubocop-clean version with one change:

def fopt
  filter = (params[:filter] || '').split(',')
  OpenStruct.new(Hash[
    %w(complete inwork cancelled ycancelled).map do |fo|
      [fo.to_sym, filter.include?(fo)]
    end
  ])
end

Note the Rubocop rule to avoid rescue in its modifier form.

| improve this answer | |
\$\endgroup\$
  • 2
    \$\begingroup\$ Changing rescue to || makes the function crash if params[:filter] is nil. However, (params[:filter] || '').split(',') could work. \$\endgroup\$ – 200_success Jan 5 '15 at 6:25
  • \$\begingroup\$ string.split(',') returns an array, which is never falsy in Ruby, so || [] will be never reached. As 200_success says, you have to do the or before the split. \$\endgroup\$ – tokland Jan 5 '15 at 22:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.