-1

I'm trying to create a new hash from params. I'm doing something like this:

where_query = { active: true }
where_query[:brand_ids] = params[:brand_ids].split(',') if params[:brand_ids].present?
where_query[:market_id] = params[:market_id] if params[:market_id].present?

Is that okay? Or is there a better way to do that?

sawa
  • 165,429
  • 45
  • 277
  • 381
Ahmed Ali
  • 2,574
  • 2
  • 23
  • 38

1 Answers1

4

I would go with more explicit declaration:

where_query =
  { 
    active: true,
    brand_ids: params[:brand_ids].try(:split, ','),
    market_id: params[:market_id]
  }.select { |_, v| v.present? }

Modern version of the above (Ruby 2.4+ required):

where_query =
  { 
    active: true,
    brand_ids: params[:brand_ids]&.split(','),
    market_id: params[:market_id].presence
  }.compact
Aleksei Matiushkin
  • 119,336
  • 10
  • 100
  • 160
  • 3
    Safe navigation is the new way to go, `params[:brand_ids]&.split(',')` instead of `try` would be nicer. – Marcin Kołodziej Sep 26 '18 at 11:19
  • @MarcinKołodziej we are already using rails (with `present?`) so I don’t see any reason to restrict my answer to the most modern versions of ruby. Many people still use 2.2- in production. Also, safe navigation effectively does the same. – Aleksei Matiushkin Sep 26 '18 at 11:28
  • 4
    I'm still kind of new when it comes to answering, I thought that if there's no specific old-version requirement, it makes sense to use the best method available. In this case, Ruby 2.3 has been out for ~3 years. While it effectively does the same, it is a bit slower and behaves differently for objects who do have the method defined (good read: https://stackoverflow.com/questions/37977721/why-is-safe-navigation-better-than-using-try-in-rails). – Marcin Kołodziej Sep 26 '18 at 11:35
  • 1
    @MarcinKołodziej well, SO is not the service for those _asking_. It’s the service for whoever will yield this page with [DuckDuckGo](https://duckduckgo.com) later. I apparently know what is the difference between `try` and `&.`, but `String#split` is surely defined and the performance here is not an issue. As I have already said, many still use ancient rubies in production (there might be dozens of reasons for that.) The answer above works for Ruby2.0/Rails2.3, so why would I make it less useful for those looking the solution for their custom environment? – Aleksei Matiushkin Sep 26 '18 at 12:04
  • Assuming that market_id is either nil or present, you could also use .compact instead of the 'select { }' part. In general I wouldn't say that the OPs way of writing it is in any way bad or worse. It doesn't look the most pretty, yet it is easy to follow. – trueunlessfalse Sep 26 '18 at 12:26
  • What if i wanted to add a nest param? i.e where_query[:id] = { not: params[:product_id] } if params[:product_id].present? how to that in the same what ? – Ahmed Ali Sep 26 '18 at 12:29
  • @AhmedAli I am not sure I follow. Just add `id: { not: params[:product_id] } if params[:product_id].present?` line inside a hash declaration. – Aleksei Matiushkin Sep 26 '18 at 12:33
  • @trueunlessfalse `Hash#compact` is even younger than `&.` (it was introduced in 2.4.) – Aleksei Matiushkin Sep 26 '18 at 12:34
  • @mudasobwa, sure, good that you mention that. I don't think that either 2.3 or 2.4 are too young to mention it in an answer on SO. I even think the default should be to provide the most up-to-date answer, and if it makes sense mention alternatives for older versions. – trueunlessfalse Sep 26 '18 at 12:53
  • @mudasobwa, sure, good that you mention that. I don't think that either 2.3 or 2.4 are too young to mention it in an answer on SO. I even think the default should be to provide the most up-to-date answer, and if it makes sense mention alternatives for older versions. – trueunlessfalse Sep 26 '18 at 12:53
  • Regarding the safe navigation: In this case String#split might be surely defined, but as you mention it yourself other people will find this answer and adapt.. a typo in try(:meothd_name) vs &.metohd_name) will only return nil vs. throwing an error, possibly making someones debugging-life harder. – trueunlessfalse Sep 26 '18 at 12:57
  • 1
    @trueunlessfalse ok, ok, I have updated the answer :) – Aleksei Matiushkin Sep 26 '18 at 13:17