CODEWITHSUNDEEP
ruby
tawheed
tawheed

Reputation: 5821

Refactoring conditional logic in ruby

I am not sure whether I am trying to abstract too much in this code or there is a simpler way to what I am trying to do. Below is the code:

some_hash = { "a" => 100, "b" => 200 } 
some_hash.each_with_index do |(key, value), index|
   if (index % 2 == 0)
      open_foo1
      open_foo2 # The order is important, that is why it has not been abstracted
   else
      open_foo2
   end
     close_foo1  # This is a bug what if foo1 was never opened, my other solution
                 # was to put it in the else clause but what if we never get there
     close_foo2
     # do something here that is applicable to both conditions
 end

So I came up with this but it just does not feel right.

 some_hash.each_with_index do |(key, value), index|
       if (index % 2 == 0)
          open_foo1
          open_foo2 # The order is important, that is why it has not been abstracted
       else
          open_foo2
       end
       if (index % 2 == 0)
         close_foo1
       end  
       close_foo2 #In the worst case foo2 would be opened
       # do something here that is applicable to both conditions
     end

Is there a better way of doing this?

Upvotes: 1

Views: 79

Answers (1)

falsetru
falsetru

Reputation: 369444

open_foo2 is called in both case; You can omit the first else.

Repeated predicate can be evaluated only once.

some_hash.each_with_index do |(key, value), index|
  foo1 = index % 2 == 0
  open_foo1 if foo1
  open_foo2
  close_foo1 if foo1
  close_foo2
end

Upvotes: 2

Related Questions