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\$
0

2 Answers 2

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)
\$\endgroup\$
2
  • \$\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
    Commented Jan 5, 2015 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
    Commented Jan 5, 2015 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.

\$\endgroup\$
2
  • 2
    \$\begingroup\$ Changing rescue to || makes the function crash if params[:filter] is nil. However, (params[:filter] || '').split(',') could work. \$\endgroup\$ Commented Jan 5, 2015 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
    Commented Jan 5, 2015 at 22:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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