我如何重构这段代码以使其干燥

Question

如何让这个代码块变干？ 我知道干意味着不要重复自己，但我没有看到任何明显的重构机会。

索引、显示、编辑和创建似乎是基本/必要的方法。它们对我来说似乎很干。

后面的方法我不太清楚。

到目前为止，除了谷歌搜索之外，我还没有尝试过任何其他方法。

class UsersController < ApplicationController

  def index
    @users = User.all
  end

  def show
    @user = User.find(params[:id])
  end

  def new
    @user = User.new
  end

  def edit
    @user = User.find(params[:id])
  end

  def create
    @user = User.new(user_params)

    respond_to do |format|
      if @user.save
        format.html { redirect_to @user, notice: 'User was successfully created.' }
        format.json { render :show, status: :created, location: @user }
      else
        format.html { render :new }
        format.json { render json: @user.errors, status: :unprocessable_entity }
      end
    end

    Slack.notify_channel 
  end

  def update
    @user = User.find(params[:id])

    respond_to do |format|
      if @user.update(user_params)
        format.html { redirect_to @user, notice: 'User was successfully updated.' }
        format.json { render :show, status: :ok, location: @user }
      else
        format.html { render :edit }
        format.json { render json: @user.errors, status: :unprocessable_entity }
      end
    end

    Slack.notify_channel
  end

  def destroy
    @user = User.find(params[:id])

    @user.destroy
    respond_to do |format|
      format.html { redirect_to users_url, notice: 'User was successfully destroyed.' }
      format.json { head :no_content }
    end

    Slack.notify_channel
  end

  private
    def user_params
      params.require(:user).permit(:username, :email)
    end
end

此代码 sn-p 没有附加 Rails 后端。我假设这只是理论上的——希望我们重构代码以使其更短。

Answer 1

恕我直言，您可以这样做。

class UsersController < ApplicationController
  include ExceptionHandling
  before_action :load_user, only: [:show, :edit, :update, :destroy]
  after_action :slack_notify_channel, only: [:create, :update, :destroy]

  def index
    @users = User.all
  end

  def new
    @user = User.new
  end

  def create
    @user = User.create!(user_params)
    respond_to do |format|
      format.html { redirect_to @user, notice: 'User was successfully created.' }
      format.json { render :show, status: :created, location: @user }
    end
  end

  def update
    @user.update!(user_params)
    respond_to do |format|
      format.html { redirect_to @user, notice: 'User was successfully updated.' }
      format.json { render :show, status: :ok, location: @user }
    end
  end

  def destroy
    @user.destroy!
    respond_to do |format|
      format.html { redirect_to users_url, notice: 'User was successfully destroyed.' }
      format.json { head :no_content }
    end
  end

  private

  def load_user
    @user = User.find(params[:id])
  end

  def slack_notify_channel
    Slack.notify_channel
  end

  def user_params
    params.require(:user).permit(:username, :email)
  end
end

我建议您创建一个关注点来管理异常并按异常呈现每个特定错误。然后，您可以避免在每个操作中使用两种方式来呈现好的和坏的情况。

Answer 2

我不得不猜测一下您的 ApplicationController 和 User 课程中的内容。但是明显重复的代码是@user = User.find(params[:id])。您的 show 和 edit 方法都只运行这一行。所以，他们做同样的事情，这是他们不应该做的。一种方法做一件事。

此外，一旦您将 show 和 edit 解析为一个方法，您的 create、update 和 destroy 方法应该调用它而不是重复该行。

接下来，我不会使用new 作为方法名，因为它已经被BasicObject::new 使用了。我测试了一些东西，结果很不清楚：

class Test
  attr_reader :test
  def initialize
    @test = 'test'
  end
end

class Test2
  attr_reader :test2
  def new
    p 'test2new'
    @test2 = Test.new
  end
end

testx = Test2.new
p testx.new.test
p testx.test2.test

=> "test2new"
=> "test"
=> "test" 

查看您何时调用自己的new 方法以及何时调用BasicObject::new 需要额外的努力。因此，如果您甚至需要它，我会将其更改为 new_user 或其他内容 - 我不明白您为什么需要它和您的 create 方法。 （如果您不需要它，请摆脱它。）

最后，@user = User.find(params[:id]) 并不意味着显示或编辑，因此了解您想要做什么并想出一个反映它的名称（例如set_user_id）也是应该做的事情.