Simplify Multiple Nil Checking in Rails

Simplify multiple nil checking in Rails

Rails has object.try(:method):

if @parent.try(:child).try(:grand_child).try(:attribute).present?
   do_something

http://api.rubyonrails.org/classes/Object.html#method-i-try

Ruby on rails, multiple check for nil attributes

You are chaining the .try(), which fails after the try(:age_id):

It tries to call age_id on the @user object
if @user.nil? # => returns nil
if @user.age_id != nil # => returns a Fixnum
Then you call the method try(:customer) on a Fixnum which obviously fails # => returns nil

etc.

An example from the IRB console:

1.9.3p448 :049 > nil.try(:nothing).try(:whatever).try(:try_this_also).nil?
 => true

If you want to test that all of these attributes are not nil, use this:

if @user.present?
  if @user.age_id.presence && @user.customer.presence && @user.country.presence
    # they are all present (!= nil)
  else
    # there is at least one attribute missing
  end
end

Check nil on multiple variables

By using none? you can have if instead of unless:

if [a,b,c,d,e,f,g].none?(&:nil?)

Come to think of it, this can be reduced to simply:

if [a,b,c,d,e,f,g].all?

if you don't mind treating false the same as nil

Is there a more efficient way of writing the following?

I think a better question is, "Is there a more expressive way of writing..."

How to simplify big conditions

First of all, some of your logic doesn't make sense:

def search(search, compare, year, rain_fall_type)
  if search == 'All'
    if rain_fall_type == 'All'
      all
    else
      # rain_fall_type != 'All'
      if year == 'All'
        if rain_fall_type == "None"
          where('Sector = ? OR Sector = ? OR Sector = ?', "Primary", 'Secondary', 'Tertiary').order('id')
        else
          where(Sector: rain_fall_type).order('id')
        end
      else
        # in rain_fall_type != 'All' branch, so meaningless 'if'
        if rain_fall_type == "All"
          order("#{year} ")
        elsif rain_fall_type == "None"
          where('Sector = ? OR Sector = ? OR Sector = ?', "Primary", 'Secondary', 'Tertiary').order('id')
        else
          where(Sector: rain_fall_type).order("#{year} ")
        end
      end
    end
  elsif compare != "None"
    # both are same, so meaningless 'if'
    if year == 'All'
      where('Sector = ? OR Sector = ?', rain_fall_type, compare).order(:id)
    else
      where('Sector = ? OR Sector = ?', rain_fall_type, compare).order(:id)
    end
  else
    # search != 'All'
    if rain_fall_type == 'All'
      all.order('id')
    else
      if year == 'All'
        if rain_fall_type == "None"
          where('Sector = ? ', search).order('id')
        else
          where('Sector = ? ', rain_fall_type).order('id')
        end
      else
        if rain_fall_type == "None"
          # in search != 'All' branch, so meaningless 'if'
          # AND both are same, so again meaningless 'if'
          if search == "All"
            where('Sector = ? ', search).order('id')
          else
            where('Sector = ? ', search).order('id')
          end
        else
          where('Sector = ? ', rain_fall_type).order('id')
        end
      end
    end
  end
end

There's more like that and I won't point it all out because we're throwing all that if stuff out, anyway.

Ultimately, we're going to defer the querying to the end of the method, like this:

def search(search, compare, year, rain_fall_type)

  ...

  @query = all 
  @query = @query.where(Sector: @sectors) if @sectors
  @query = @query.order(@order) if @order
  @query

end

That way, you take all of your where and order statements, and do them only once at the end. That saves a lot of typing right there. See the comment from muistooshort for why (Sector: @sectors) works.

So, the trick is setting @sectors and @order. First, I'm going to assign the input variables to instance variables because I like it like that (and to avoid confusion between the variable @search and the method search):

def search(search, compare, year, rain_fall_type)
  @search, @compare, @year, @rain_fall_type = search, compare, year, rain_fall_type

  ...

  @query = all 
  @query = @query.where(Sector: @sectors) if @sectors
  @query = @query.order(@order) if @order
  @query
end

Now, this answer is going on too long already, so I won't drag you through all the gorey details. But, adding in a couple of helper methods (sectors_to_use, and order_to_use) and substituting them in for @sectors and @order, you basically end up with this:

def search(search, compare, year, rain_fall_type)
  @search, @compare, @year, @rain_fall_type = search, compare, year, rain_fall_type
  @query = all 
  @query = @query.where(Sector: sectors_to_use) if sectors_to_use
  @query = @query.order(order_to_use) if order_to_use
  @query
end

private 

def sectors_to_use
  return [@rain_fall_type, @compare] if @search != 'All' && @compare != 'None'
  unless @rain_fall_type == 'All'
    if @rain_fall_type == 'None'
      @search == 'All' ? ['Primary', 'Secondary', 'Tertiary'] : [@search]
    else
      [@rain_fall_type]
    end  
  end
end

def order_to_use
  return nil if (@search == 'All') && (@rain_fall_type == 'All')
  return @year if (@search == 'All') && !(@year == 'All')
  return :id
end

That's less than half the lines of code, over a thousand fewer characters, and a whole lot fewer ifs.

What is a better way to check for a nil object before calling a method on it?

Personally, I would use the or operator/keyword:

(financial_document.assets or []).length

Either way, .length is called on an array, giving you 0 if nil.

How to simplify big conditions

First of all, some of your logic doesn't make sense:

def search(search, compare, year, rain_fall_type)
  if search == 'All'
    if rain_fall_type == 'All'
      all
    else
      # rain_fall_type != 'All'
      if year == 'All'
        if rain_fall_type == "None"
          where('Sector = ? OR Sector = ? OR Sector = ?', "Primary", 'Secondary', 'Tertiary').order('id')
        else
          where(Sector: rain_fall_type).order('id')
        end
      else
        # in rain_fall_type != 'All' branch, so meaningless 'if'
        if rain_fall_type == "All"
          order("#{year} ")
        elsif rain_fall_type == "None"
          where('Sector = ? OR Sector = ? OR Sector = ?', "Primary", 'Secondary', 'Tertiary').order('id')
        else
          where(Sector: rain_fall_type).order("#{year} ")
        end
      end
    end
  elsif compare != "None"
    # both are same, so meaningless 'if'
    if year == 'All'
      where('Sector = ? OR Sector = ?', rain_fall_type, compare).order(:id)
    else
      where('Sector = ? OR Sector = ?', rain_fall_type, compare).order(:id)
    end
  else
    # search != 'All'
    if rain_fall_type == 'All'
      all.order('id')
    else
      if year == 'All'
        if rain_fall_type == "None"
          where('Sector = ? ', search).order('id')
        else
          where('Sector = ? ', rain_fall_type).order('id')
        end
      else
        if rain_fall_type == "None"
          # in search != 'All' branch, so meaningless 'if'
          # AND both are same, so again meaningless 'if'
          if search == "All"
            where('Sector = ? ', search).order('id')
          else
            where('Sector = ? ', search).order('id')
          end
        else
          where('Sector = ? ', rain_fall_type).order('id')
        end
      end
    end
  end
end

There's more like that and I won't point it all out because we're throwing all that if stuff out, anyway.

Ultimately, we're going to defer the querying to the end of the method, like this:

def search(search, compare, year, rain_fall_type)

  ...

  @query = all 
  @query = @query.where(Sector: @sectors) if @sectors
  @query = @query.order(@order) if @order
  @query

end

def search(search, compare, year, rain_fall_type)
  @search, @compare, @year, @rain_fall_type = search, compare, year, rain_fall_type

  ...

  @query = all 
  @query = @query.where(Sector: @sectors) if @sectors
  @query = @query.order(@order) if @order
  @query
end

def search(search, compare, year, rain_fall_type)
  @search, @compare, @year, @rain_fall_type = search, compare, year, rain_fall_type
  @query = all 
  @query = @query.where(Sector: sectors_to_use) if sectors_to_use
  @query = @query.order(order_to_use) if order_to_use
  @query
end

private 

def sectors_to_use
  return [@rain_fall_type, @compare] if @search != 'All' && @compare != 'None'
  unless @rain_fall_type == 'All'
    if @rain_fall_type == 'None'
      @search == 'All' ? ['Primary', 'Secondary', 'Tertiary'] : [@search]
    else
      [@rain_fall_type]
    end  
  end
end

def order_to_use
  return nil if (@search == 'All') && (@rain_fall_type == 'All')
  return @year if (@search == 'All') && !(@year == 'All')
  return :id
end

That's less than half the lines of code, over a thousand fewer characters, and a whole lot fewer ifs.

Simplify multiple if else condition in rails model

def set_status_of_day(late_policy, early_departure_policy)
  case [late_policy.warning_on_late, early_departure_policy.warning_on_late]
    when ["Half Day", "Half Day"] then "Absent"
    when ["Half Day", "Present"], ["Half Day", "Early Departure"], ["Late", "Early Departure"] then "Half Day"
    when ["Present", "Present"] then "Present"
    .
    .
    .
  end
end

I added the second when-line like Cary Swoveland suggested in the comments. The commas in the when clause work like an or-conjuction.
See case in the docs for mor information.

ActiveRecord::Relation issue checking for nil? -- Rails 3.1

use blank?, nil? is true, if it's really nil (single instance of NilClass), but your second example always will return an Array, maybe empty, if there are no results, but an Array nonetheless. blank? checks for empty arrays, empty strings, nil and false values.

If you have problems with blank? not behaving as expected you can check for first.nil?

Simplify Multiple Nil Checking in Rails