Ruby array reject elements based on condition

Question

I am trying to reject array items based on multiple conditions.

The code is as follows

      def fetch_items
        items= line_items.reject(&driving?)
        if new_order_history_enabled?
          items = items.reject{ |li| li.expenses == 0 }
        end
        items
     end
     def driving?
        proc { |line_item| LineItemType.new(line_item, segment).drive? }
     end

Is there a one liner or a more cleaner way to write this?

Something like

items= line_items.reject { |li| li.driving? && ( new_order_history_enabled? && li.expenses == 0)}

"Something like" - doesn't this work?

Sergio Tulentsev
– Sergio Tulentsev

2021-08-10 12:03:25 +00:00
Commented Aug 10, 2021 at 12:03 — Sergio Tulentsev
– Sergio Tulentsev, Commented Aug 10, 2021 at 12:03

PCurell · Accepted Answer · 2021-08-10 12:10:43Z

3

items= line_items.reject { |li| li.driving? || (new_order_history_enabled? && li.expenses == 0)}

Since you want both to apply here, I think you should use || instead of &&

That way, you are actually doing what you describe in your method. (and you only iterate once over the array, which is cool :) )

Although, and this is stylistic preference. I would prefer to do:

items = line_items.reject do |li| 
  li.driving? || 
    (new_order_history_enabled? && li.expenses == 0)
end

since it might be clearer at a glance what we are doing

answered Aug 10, 2021 at 12:10

PCurell

1113 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Les Nightingill · Accepted Answer · 2021-08-11 05:03:48Z

1

Personally I don't think a one-liner is always cleaner, especially when it's a long one-liner. The style that (to me) is cleaner, is to write:

  def fetch_items
    items= line_items.reject(&:driving?)
    items= items.reject(&:zero_expenses?) if new_order_history_enabled?
  end

  def driving?
    proc { |line_item| LineItemType.new(line_item, segment).drive? }
  end

# in the LineItem class, define the zero_expenses? method:
  def zero_expenses?
    expenses.zero?
  end

answered Aug 11, 2021 at 5:03

Les Nightingill

6,1742 gold badges32 silver badges34 bronze badges

Collectives™ on Stack Overflow

Ruby array reject elements based on condition

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related