rails，Rubocop重构

Question

犯罪： app/控制器/任务_控制员.12:12:3: C:度量/摘要大小:创建的分配分支条件大小太高。<3，19，1> 19.26/17 def创建 应用程序/控制器/任务_控制器. is :24:3: C:度量/摘要大小:分配分支条件的更新大小太高。<4，20，1> 20.42/17 def更新 检查了27份档案，发现2项违法行为

  def create
    @task = current_user.projects.find(params[:project_id]).tasks.new
    @task.title = task_params[:title]

    @task.save ? (redirect_to root_url) : (flash[:error] = @task.errors.full_messages) | (render 
    :new)
  end

  def update
    @task = current_user.tasks.find(params[:id])
    @task.title = task_params[:title]
    @task.deadline = task_params[:deadline]

    @task.save ? (redirect_to root_url) : (flash[:error] = @task.errors.full_messages) | (render 
    :edit)
  end

我怎么才能重建它来解决盗贼的罪行呢？

通过创建私有方法解决问题：

  def create
    tasknew
    tasktitle

    @task.save ? (redirect_to root_url) : (flash[:error] = @task.errors.full_messages) | (render :new)
  end

  def update
    taskfind
    tasktitle
    @task.deadline = task_params[:deadline]

    @task.save ? (redirect_to root_url) : (flash[:error] = @task.errors.full_messages) | (render :edit)
  end

private

  def tasktitle
    @task.title = task_params[:title]
  end

  def taskfind
    @task = current_user.tasks.find(params[:id])
  end

  def tasknew
    @task = current_user.projects.find(params[:project_id]).tasks.new
  end

Answer 1

您提取了代码中最简单的部分，并留下了最复杂的部分。ABC度量的目的是指出代码的复杂部分，以便简化它们，并可能捕获bug。如果你只提取最琐碎的部分来满足警察，你也可以禁用它。

显然，@task.save ? (redirect_to root_url) : (flash[:error] = @task.errors.full_messages) | (render :edit)是这些方法中最复杂的部分。我很难理解。为什么有一个按位或？它是复制的，除了或渲染什么。

我们把它拆开。首先，将三元操作符转换为if/else。

if @task.save
  redirect_to root_url
else
  (flash[:error] = @task.errors.full_messages) | (render :edit)
end

我不认为|真的在做任何事情。这只是把两个陈述塞进一个里的一种方式。

if @task.save
  redirect_to root_url
else
  flash[:error] = @task.errors.full_messages
  render :edit
end

这是更容易理解，并减少了一点ABC。

接下来是将查找代码放入自己的私有方法中。只是一个简单的提取方法。

# Note I left it at just finding the associated project.
# I left the .tasks.new off because that's very specific.
private def current_project
  current_user.projects.find(params[:project_id])
end

private def current_task
  current_user.tasks.find(params[:id])
end

def create
  @task = current_project.tasks.new
  @task.title = task_params[:title]

  if @task.save
    redirect_to root_url
  else
    flash[:error] = @task.errors.full_messages
    render :edit
  end
end

def update
  @task = current_task
  @task.title = task_params[:title]
  @task.deadline = task_params[:deadline]

  if @task.save
    redirect_to root_url
  else
    flash[:error] = @task.errors.full_messages
    render :edit
  end
end

这样就行了。您可以更进一步，提取保存逻辑，传入呈现的位置和可能更改的任何其他细节。

# Note the use of defaults.
private def save_task(render:, redirect_to: root_url)
  if @task.save
    redirect_to(redirect_to)
  else
    flash[:error] = @task.errors.full_messages
    render(render)
  end
end

def create
  @task = current_project.tasks.new
  @task.title = task_params[:title]

  save_task(render: :new)
end

def update
  @task = current_task
  @task.title = task_params[:title]
  @task.deadline = task_params[:deadline]

  save_task(render: :edit)
end

它揭示了一些可能的问题。

create和update正在使用不同的方法来查找任务。看上去很可疑。您的tasknew和taskfind隐藏了这种差异，使得代码更难理解。

update正在设定最后期限，但create没有。这意味着应该将task_params中的@任务属性提取到一个方法中。

请注意，我避免将赋值放入私有方法中。尽管这在技术上减少了ABC，但它使代码更难理解，因为方法调用有副作用((computer_science%29)；它改变了对象的状态)。副作用必须有的放矢。

例如，可以将current_task和current_project转换为带有默认值的回忆录访问器。

private def current_project
  @current_project ||= current_user.projects.find(params[:project_id])
end

现在，它将只加载对象一次。