Ruby reverse.each_with_index и delete_at, вызывающие проблемы в последних Ruby/Rails

Question

Итак, я хорошо знаю об опасностях удаления элементов в итерационном блоке (это цикл в обратном порядке), и я знаю, что Матц упомянул кое-что о мутациях в итерациях, вызывающих проблемы со стабильностью, но я не могу понять это,

Этот пример немного запутан, и я не уверен, что даже его решение точно воспроизведет пример, но я должен попробовать.

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

Выходы:

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

когда это должно быть:

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

изменение delete_at(index) на delete(j) исправляет это, но не работало, когда массив был объектами. Я также копирую объекты во временный массив, чтобы усложнить задачу.

В моем реальном сценарии у меня есть два массива, заполненных объектами модели другого типа, но с общим атрибутом (возможно, здесь можно использовать соединение, но я пытаюсь избежать специальной таблицы соединений). Я хочу удалить все объекты в массиве1, которые имеют общий атрибут в массиве2. Я пробовал много разных вещей без решения... слишком много, чтобы положить сюда.

@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

Без дополнительного копирования, приведенного выше, связи моделей останутся, и в итоге я удалю запись из таблицы, чего не должно быть. Это просто временный список. Это кажется глупой проблемой, и я потратил слишком много времени на это.

Answer 1

Вы удаляете, используя обратный индекс, но из исходного массива.

Чтобы получить "реальный" индекс, вместо одного отсчета от конца массива, вам нужно перевернуть его:

arr1.delete_at(-index - 1)

---

... но вы почти наверняка должны использовать reject! или же delete_if вместо:

require "set"

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

Answer 2

Есть несколько решений для вашей проблемы, чистый пример показан ответом @matthewds. Однако два решения ниже также решают вашу проблему.

Прежде всего я хотел сообщить вам, что вы можете сократить первые несколько строк кода:

@arr1 = []
original_arr1 = Model1.where(...)
original_arr1.each { |original| @arr1 << original.dup }
# to
@arr1 = Model1.where(...).map(&:dup)
# but since you're not saving the Model1.where(...) result in a variable
# (enabling one to use them later), there is not need to dup at all
@arr1 = Model1.where(...)

Проблема

Фактический результат, который вы получаете, верен. Вот почему:

a1 = [1, 2]
a2 = [2]

a2.each { |n2| a1.reverse.each_with_index { |n1, i| a1.delete_at(i) if n2 == n1 }  }

# a1 = [1, 2]
# a2 = [2]
# iterate over a2
# n2 = 2
# create an new array with the reversed elements of a1
# ra1 = a1.reverse (eq [2, 1] and a1 is still [1, 2])
# iterate over ra1 with index
# n1 = 2, i = 0
# does n2 (2) equals n1 (2)? yes
# delete in a1 ([1, 2]) at the index i (0)
# resulting in a1 = [2]
# next iteration ra1
# n1 = 1, i = 1
# does n2 (2) equals n1 (1)? no
# ra1 iteration finishes
# a2 iteration finishes
# resulting in a1 = [2]

# 1 Сохранение вашей текущей структуры кода

Если вы хотите самое простое решение для вашей текущей структуры кода, просто удалите #reverse звонка должно хватить. Кажется, что нет необходимости переворачивать массив в любом случае, так как вы не сохраняете результат или не используете в #each_with_index кодовый блок.

# 2 Выбирайте только те записи, которые вам нужны

Это второе решение решает проблему на уровне запросов к базе данных. Если вы не хотите, чтобы записи из Model1 с тем же цветом, что и цвета в вашем текущем наборе, затем не извлекайте их из базы данных.

@arr2 = Model2.where(...)
@arr1 = Model1.where(...).where.not(color: @arr2.pluck(:color))

Если цвет не атрибут, а экземпляр ассоциации, используйте :color_id вместо.

Примечание: вы можете использовать .select(:color) вместо .pluck(:color), Использование метода select приведет к подзапросу, так как вы, вероятно, собираетесь использовать @arr2 в любом случае записи должны быть загружены полностью. Срывая значения с @arr2 и предоставление их в виде простых цветов вместо подзапроса сохраняет базу данных некоторую работу. Если вы не собираетесь использовать @arr2 дальше я бы использовал выбранный вариант.

Answer 3

Я не тестировал его со сложными структурами данных, но, возможно, это могло быть другим способом.

a = [1, 2, 3, 4, 5]
b = [3, 4, 5]

p a+b-(a&b)
p a&b

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

Который работает также когда:

a = [3, 4, 5]
b = [1, 2, 3, 4, 5]