0
votes
def main_method
    new_array = []

    some_array.each do |foo|
      if (method_01? foo) || (method_02? foo) || (method_03? foo) || (method_04? foo) || (method_05? foo)
      else
        new_array << foo
      end
    end
  end

Is there a better way to write the above code without or (||) and without elsif conditions?

Is looping thru a hash appropriate for such refactoring?

3
Each method (method_01?, method_02?...) contains a calculation that edits the foo and adds back to new_array if none of the methods match then the element is added the to the new_array - Баста
It is rather really practice to define questionmark-methods which have side-effects, i.e. which change data outside of their local method scope. This is generally unexpected and can cause you a lot of grief later. - Holger Just

3 Answers

2
votes

Maybe something like this would help?

I've updated my answer for splitting array on two groups.

def main_method
  methods = [:method1, :method2, :method3]
 non_passed_elems, passed_elems = some_array.partition do |elem|
    methods.none? do |method|
      send(method, elem)
    end
  end
  passed_elems.each{ |t| method_for_passed_elems(t) }
  non_passed_elems.each{ |t| method_for_non_passed_elems(t) }
end
0
votes

It would be hard to come up with an alternative to the ||s without knowing your specific use case, but the iteration can be cleaned up a good deal using #reject:

def main_method
  some_array.reject do |foo|
    method_01?(foo) || method_02?(foo) # ... etc.
  end
end

#reject will yield each member to the block and return an array that contains only the members that returned false. In other words, only the members that return false to all of methods 1-5.

0
votes

More concise way:

checkers = (1..5).map {|i| "method_%02d" % i}  # too lazy ;)
new_array = some_array.select {|e| !checkers.any? {|m| e.send m}}

Still pretty readable & obvious.