9

I have the following very ugly ruby code in a rails app I'm working on:

if params.present?
  if params[:search].present?
    if params[:search][:tags_name_in].present?
      ...
    end
  end
end

All I'm trying to ask is whether params[:search][:tags_name_in] has been defined, but because params, and params[:search], and params[:search][:tags_name_in] might all be nil, if I use...

if params[:search][:tags_name_in].present?

... I get an error if there are no params or no search params.

Surely there must be a better way to do this... suggestions??

Brett Bender
  • 19,388
  • 2
  • 38
  • 46
Andrew
  • 42,517
  • 51
  • 181
  • 281
  • A note about which answer I picked: In my app its not important whether this condition returns `nil` or `false`, nor is it important that `:tags_name_in` not be blank, I just need to test if `:tags_name_in` is defined without raising an error. So, for my situation, I like the `defined?` approach as given by Will Ayd. However, I think Mike Lewis approach may be more useful for others who may need to avoid passing a nil value into their condition. So, please look at both answers and decide whether that matters in your situation. – Andrew Mar 22 '11 at 17:03

6 Answers6

11

if you are just trying to see if its defined why not keep it simple and use the defined? function?

if defined?(params[:search][:tags_name_in])
Will Ayd
  • 6,767
  • 2
  • 36
  • 39
  • 2
    This is not equivalent to the code he originally asked regarding. `defined?` will return true if a variable has been initialized, even if it's set to `nil`. `present?` checks using `blank?`, so `nil` will return false. – Brett Bender Mar 22 '11 at 16:14
  • sure but the question says he is just asking if it has been defined or not - which this will do. – Will Ayd Mar 22 '11 at 16:16
  • This is the cleanest answer, it also happens to be wrong. – Brett Bender Mar 22 '11 at 16:16
  • so clarification on the question then - are you looking for defined or not nil? – Will Ayd Mar 22 '11 at 16:17
  • Ok - after testing in my particular case defined actually works perfectly. There is no situation in my app where :tags_name_in could be defined and nil -- but even if it was defined and nil that doesn't raise any errors, just nothing happens (which is the desired behavior). So, I like this approach for my situation. – Andrew Mar 22 '11 at 16:53
  • 1
    @Andrew If `params[:search][:tags_name_in]` is nil, `defined?` will not throw any errors, it will return the string "local-variable" or some such, which, to an `if` statement, is true. Just because it happens to work right now, for you, does not make it a good solution. The point is that the behavior of defined? is not immediately obvious or intuitive, and in the future you or the person taking over your code may get bitten badly. Using this approach sacrifices maintainability in order to save 9 characters. – Brett Bender Mar 22 '11 at 18:15
  • the other problem is that to use this value, you'll have to write that string of symbols and brackets over again, creating a second point of failure and violating DRY. – Ben Wheeler Mar 13 '15 at 06:33
6

Params will always be defined, so you can remove that.

To reduce the amount of code you can do

if params[:search] && params[:search][:tags_name_in]
  #code
end

If params[:search] is not defined, the condition will short circuit and return nil.

Marc-André Lafortune
  • 78,216
  • 16
  • 166
  • 166
Mike Lewis
  • 63,433
  • 20
  • 141
  • 111
4

You can use andand for this. It handles this exact situation:

if params[:search].andand[:tags_name_in].andand.present?

ryeguy
  • 65,519
  • 58
  • 198
  • 260
  • This is a very cool gem, thanks for the tip! – Andrew Mar 22 '11 at 16:54
  • +1 if only everybody used andand in these cases (though I prefer to write the equivalent ick's "maybe") instead of conditionals and more conditionals (or the ugly "try") – tokland Mar 22 '11 at 17:22
4

You have many choices that will return the value of params[:search][:tags_name_in] or nil if params[:search] is nil.

Clear but lengthy:

params[:search] && params[:search][:tags_name_in]

Using try (from active_support):

params[:search].try(:[], :tags_name_in)

Using rescue:

params[:search][:tags_name_in] rescue nil

Using fetch:

params.fetch(:search, {})[:tags_name_in]

Note that fetch can sometime be used to avoid the if altogether, in particular if there is nothing to do when the param is not specified:

def deal_with_tags
  MyModel.where :tags => params.fetch(:search){ return }[:tags_name_in]
end
Marc-André Lafortune
  • 78,216
  • 16
  • 166
  • 166
  • +1. I was just about to post the `try` and `fetch` solution. I have stopped using inline `rescue` after finding the cost of exception handling in Ruby. – Harish Shetty Mar 22 '11 at 17:02
  • The fetch method looks very useful, I'll look it up and I may be able to use it in several places! – Andrew Mar 22 '11 at 17:05
  • OP needs to deal with params itself being nil! This answer would still throw errors in that case. – Ben Wheeler Mar 13 '15 at 06:24
3

Haha, if you want to be terrible and monkeypatch nil:

class NilClass
  def [](key)
    nil
  end
end

I would recommend a short-circuited if like the other answers suggest, though.

Nathan Ostgard
  • 8,258
  • 2
  • 27
  • 19
0

I usually end up doing something like this:

if params.has_key?(:search) && params[:search].has_key?(:tags_name_in)
...

end

Although if you don't mind testing against nils in your if statement you could also do this:

if params[:search] && params[:search][:tags_name_in] ...

This will not throw an error because ruby short-circuits the && operator.

Brett Bender
  • 19,388
  • 2
  • 38
  • 46