3

These params come out of html inputs in erb templates (this code is in the main application.rb), and I am checking if they are filled before I add them to n.requestusers, which will become part of a database entry. It works, but it feels more like a bash script the way it is now. What would be the best way to write something like this?

a route in the main .rb

if params[:user2].empty? && params[:user3].empty? && params[:user4].empty? && params[:user5].empty?
  n.requestusers = params[:user1]
elsif params[:user3].empty? && params[:user4].empty? && params[:user5].empty?
  n.requestusers = params[:user1], params[:user2]
elsif params[:user4].empty? && params[:user5].empty?
  n.requestusers = params[:user1], params[:user2], params[:user3]
elsif params[:user5].empty?
  n.requestusers = params[:user1], params[:user2], params[:user3], params[:user4]
else
  n.requestusers = params[:user1], params[:user2], params[:user3], params[:user4], params[:user5]
end
1
  • Would you like to share your view? There might be a better way to do this than by sending a bunch of params[:userX]. Maybe a params[:users] ? May 7, 2013 at 21:44

3 Answers 3

6

Instead of having all of those conditional statements might you be interested in something like:

n.requestusers = params.select { |key, val| not val.empty? }.values

Or a cleaner way as suggested by @theTinMan:

n.requestusers = params.reject { |key, val| val.empty? }.values

select lets you take all of the none empty parameter values and returns them. values lets you grab those values as an array.

I am not experienced with web frameworks, so my suggestion is a bit of a shot in the dark.

4
  • 1
    +1 Two things to make it more Ruby-like. In general, instead of not, use !. For this code, instead of select, use reject { |k, v| v.empty? } because not or ! makes us flip the logic in our heads. Switching to reject relies on the the success of the empty? test. May 7, 2013 at 23:20
  • @theTinMan I agree with you. I completely overlooked reject. That is much more intuitive.
    – squiguy
    May 7, 2013 at 23:26
  • 2
    I notice those things because I don't like my head hurting. :-) May 7, 2013 at 23:27
  • Yes! This is just the kind of thing I was looking for! :) Thanks!
    – jerius
    May 8, 2013 at 14:26
1

This isn't tested, because there are no sample values to test against, but, after some refactoring I have:

if [:user2, :user3, :user4, :user5].all?{ |s| params[s].empty? }
  n.requestusers = params[:user1]
elsif [:user3, :user4, :user5].all? { |s| params[s].empty? }
  n.requestusers = [:user1, :user2].map{ |s| params[s] }
elsif [:user4, :user5].all? { |s| params[s].empty? }
  n.requestusers = [:user1, :user2, :user3].map{ |s| params[s] }
elsif params[:user5].empty?
  n.requestusers = [:user1, :user2, :user3, :user4].map{ |s| params[s] }
else
  n.requestusers = [:user1, :user2, :user3, :user4, :user5].map{ |s| params[s] }
end

Looking at that further, this seems sensible:

USER_LIST = [:user1, :user2, :user3, :user4, :user5]
USER_LIST.size.times do |i|
  user_list = USER_LIST
  get_users = user_list.shift(1 + i)
  if user_list.all?{ |s| params[s].empty? }
    n.requestusers = params.values_at(get_users)
    break
  end
end

Like I said, that's not tested, but I'd work with something along those lines.

Adjust USER_LIST as necessary.

0

I am not that familiar with Sinatra, but if you wanted to add elements to the array, you can just do (assuming n.requestusers has already been initialized):

n.requestusers << params[:user1] unless params[:user1].empty?

You can do that for each user parameter.

Edit: It would probably be better just to check if the param exists since if it isn't found, nil is returned -- calling empty? on nil throws a NoMethodError. It would probably better to do:

n.requestusers << params[:user1] unless params[:user1]

This would ensure that it adds the param if it exists.

1
  • In case you do not know, unless translates to if not.
    – TheDude
    May 7, 2013 at 21:36

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.