重构LIst排序方法的控制器代码

Question

使用排序列进行列表排序。函数运行良好，但是我觉得必须有一种方法来重构这段代码，因为唯一的区别是一些变量赋值的运算符！

方法过去是分开的，所以设法稍微减少了代码，但还不够！

一个方法将列表中的项向上移动，而向下方法将列表项向下移动一个。交换方法是切换列表中的位置。

我在model作用域中设置了顺序

  def up
    return false if @dish == @section.dishes.first
    swap('up', @dish, @section)
  end

  def down
    return false if @dish == @section.dishes.last
    swap('down', @dish, @section)
  end

  def swap(direction, dish, section)
    dish_i = 0
    section.dishes.each_with_index do |d, i|
      dish_i = i - 1 if dish == d && direction == 'up'
      dish_i = i + 1 if dish == d && direction == 'down'
    end
    dish2 = section.dishes[dish_i]
    if direction == 'up'
      dish2.sort += 1
      dish.sort -= 1
    elsif direction == 'down'
      dish2.sort -= 1
      dish.sort += 1
    end
    if @dish.save && dish2.save
      redirect_to menu_path(params[:menu_id])
    else
      render :show
    end
  end

我想减少代码，因为很多逻辑都是一样的

Answer 1

要做到这一点，最简单的方法是使用acts_as_list gem，它是成熟的、经过良好测试的、专门为此目的而设计的。

也就是说，我认为您可能稍微想多了一点。您不需要执行所有的数学运算，也不需要获取本节中的所有其他菜。你所需要做的就是用下一个更高/更低的sort来获取碟子，并交换两个碟子的sort：

def up
  swap!(:up, @dish, @section)
end

def down
  swap!(:down, @dish, @section)
end

def swap!(direction, dish, section)
  condition = direction == :up ? 'sort > ?' : 'sort < ?'
  asc_or_desc = direction == :up ? :desc : :asc
  next_dish = section.dishes
    .where(condition, dish.sort)
    .order(sort: asc_or_desc)
    .first

  return false if next_dish.nil?

  Dish.transaction do
    next_dish.update!(sort: dish.sort)
    dish.update!(sort: next_dish.sort)
  end
end

关于组织代码的主题，代码redirect_to和render代码应该放在你的控制器方法中(例如update)，而不是swap。就此而言，上面的所有代码可能都属于你的模型，而不是你的控制器：

class Dish < ApplicationRecord
  # ...

  def move_up!(section)
    swap!(:up, section)
  end

  def move_down!(section)
    swap!(:down, section)
  end

  def swap!(direction, section)
    condition = direction == :up ? 'sort > ?' : 'sort < ?'
    asc_or_desc = direction == :up ? :desc : :asc
    next_dish = section.dishes
      .where(condition, sort)
      .order(sort: asc_or_desc)
      .first

    return false if next_dish.nil?

    transaction do
      next_dish.update!(sort: sort)
      update!(sort: next_dish.sort)
    end
  end
end

然后在你的控制器方法中，你只需要调用例如@dish.move_up!(@section)。

Answer 2

可能有一种更简单的形式，但这种通用方法可能是开始的基础。这使用了数组修饰符，其中数组的块可以被给予reverse处理来交换它们的顺序：

def swap(list, el, dir)
  index = list.index(el)
  list = list.dup

  case (dir)
  when :up
    # Do simple bounds checking here
    unless (index > 0)
      return list
    end

    # Offset the swap
    index -= 1

  when :down
    unless (index + 1 < list.length)
      return list
    end
  end

  # Reverse the elements at the target location
  list[index, 2] = list[index, 2].reverse

  list
end

您需要将render和redirect_to调用排除在swap方法之外，以便它只做一件事。你通过让它一次做一堆事情而使方法过于复杂，这使得测试变得困难。

这里可以很容易地演示这个：

list = %w[ a b c d e f ]

p swap(list, 'a', :down) # => ["b", "a", "c", "d", "e", "f"]
p swap(list, 'a', :up)   # => ["a", "b", "c", "d", "e", "f"]
p swap(list, 'c', :down) # => ["a", "b", "d", "c", "e", "f"]
p swap(list, 'c', :up)   # => ["a", "c", "b", "d", "e", "f"]
p swap(list, 'f', :down) # => ["a", "b", "c", "d", "e", "f"]
p swap(list, 'f', :up)   # => ["a", "b", "c", "d", "f", "e"]