Search code examples
ruby-on-railsrubyruby-on-rails-5ruby-2.5

Ruby reverse.each_with_index and delete_at causing problems in latest Ruby/Rails


So, I'm well aware of the perils of removing items in an iteration block (this is looping in reverse), and I know Matz mentioned something about mutations in iterations causing stability problems, but I can't seem to figure this one out.

This example is a little convoluted and I'm not sure even solving it will replicate the example exactly, but I'll have to try.

arr1 = [1, 2, 3, 4, 5]
arr2 = [3, 4, 5]
puts arr1.inspect
puts arr2.inspect
arr2.each do |i|
  arr1.reverse.each_with_index do |j, index|
    if i == j
      arr1.delete_at(index)
    end
  end
end
puts arr1.inspect
puts arr2.inspect

Outputs:

[1, 2, 3, 4, 5]
[3, 4, 5]
[4, 5]
[3, 4, 5]

when it should be:

[1, 2, 3, 4, 5]
[3, 4, 5]
[1, 2]
[3, 4, 5]

changing delete_at(index) to delete(j) fixes this, but didn't work when the array was objects. I'm also copying the objects to a temp array to make matters more complicated.

In my real life scenario, I have two arrays filled with Model objects of different type but share a common attribute (could probably use a join here, but I'm trying to avoid a special join table). What I want is to remove any objects in array1 that have a common attribute in array2. I've tried a number of different things with no solution... too many to put here.

@arr1 = []
original_arr1 = Model1.where(...)
original_arr1.each { |original| @arr1 << original.dup }
@arr2 = Model2.where(...)
@arr2.each do |object1|
  @arr1.reverse.each_with_index do |object2, index|
    if object1.color == object2.color
      @arr1.delete_at(version_index)
    end
  end
end

Without the extra copying above, the Model associations will remain and I will end up deleting the record from the table, which should not happen. It's just a temporary list. This seems like a stupid problem and I've spent way too much time on it.


Solution

  • You're deleting using the reversed index, but from the original array.

    To get the "real" index, instead of one counting from the end of the array, you need to flip it around:

    arr1.delete_at(-index - 1)
    

    ... but you should almost certainly be using reject! or delete_if instead:

    require "set"
    
    unwanted_colors = @arr2.map(&:color).to_set
    @arr1.reject! { |el| unwanted_colors.include?(el.color) }