CODEWITHSUNDEEP
rubynested-if
rlhh
rlhh

Reputation: 903

Ruby: nested if statements

I was writing some code and it ended up being way too ugly to my liking. Is there anyway that I can refactor it so that I don't use nested if statements?

def hours_occupied(date)
  #assuming date is a valid date object    
  availability = get_work_hours(date)
  focus = "work"

  if availability.nil
    availability = get_family_hours(date)
    focus = "family"

    if availability.nil
      availability = get_friend_hours(date)
      focus = "friends"
    end
  end
end

I know I'll be able to do something like this for availability

availability = get_work_hours(date) || get_family_hours(date) || get_friend_hours(date)

but how do I set the focus variable accordingly?

Upvotes: 5

Views: 29395

Answers (4)

tokland
tokland

Reputation: 67850

I'd write:

def hours_occupied(date)
  focus = if (availability = get_work_hours(date))
    "work"
  elsif (availability = get_family_hours(date))
    "family"
  elsif (availability = get_friend_hours(date))
    "friends"
  end
  # I guess there is more code here that uses availability and focus.
end

However, I am not sure having different methods for different types is a good idea, it makes code harder to write. A different approach using Enumerable#map_detect:

focus, availability = [:work, :family, :friends].map_detect do |type|
  availability = get_hours(date, type)
  availability ? [type, availability] : nil
end

Upvotes: 3

trushkevich
trushkevich

Reputation: 2677

One more way is just to reassign values if there is a need:

def hours_occupied(date)
  availability, focus = get_work_hours(date), "work"
  availability, focus = get_family_hours(date), "family" unless availability
  availability, focus = get_friend_hours(date), "friend" unless availability
end

or using an iterator:

def hours_occupied(date)
  availability = focus = nil
  %w(work family friend).each {|type| availability, focus = self.send(:"get_#{type}_hours", date), type unless availability}
end

Upvotes: 2

steenslag
steenslag

Reputation: 80065

A case when is also an option:

focus = case availability
when get_work_hours(date)
  "work"
when get_family_hours(date)
  "family"
when get_friend_hours(date)
  "friends"
end

Upvotes: 1

Don Cruickshank
Don Cruickshank

Reputation: 5938

I would do something like the following as it makes it clear that each case is mutually exclusive:

def hours_occupied(date)
  if availability = get_work_hours(date)
    focus = "work"
  elsif availability = get_family_hours(date)
    focus = "family"
  elsif availability = get_friend_hours(date)
    focus = "friends"
  end
end

Upvotes: 7

Related Questions