Rails リファクタリングへの意識

#はじめに
ポートフォリオがひとまず形になりrspecも書いたため、リファクタリングを行った際、
コントローラのアクション1つだけでも考える事がたくさんあったため、それをまとめたものです。
まだまだ最適解では無いかもしれないため、いい方法があれば誰か教えていただけると幸いです。

#リファクタリング前のコード
下記は企業登録を行うアクションとなります。
簡単に流れを説明すると、
フォームから送信された物が問題なければ企業を登録し、従業員を一人追加するアクションとなります。

コントローラ

  def create
    @company = Company.new(company_params)
    #従業員の中に同じEメールが無いかチェックし、いたら登録をせずフォームに返す
    if Employee.find_by(email: @company.email)
      @company.errors.add(:email, "はすでに存在します")
      return render 'new'
    end
    @company.usage_status = true
    if @company.save
      #企業登録時に、1人管理者権限を持った従業員を登録
      @company.employees.create(name: self.responsible_name,
                                email: self.email,
                                password: 'password',
                                department: '管理者',
                                joining_date: Date.today,
                                admin: true,
                                enrollment_status: true)
      flash[:success] = '登録完了しました。'
      redirect_to admin_companies_path
    else
      render 'new'
    end
  end

#① @company.employees.createをモデルにて定義
コントローラの中で行数が多い記述となっており見栄えが悪かったため、モデル内に記載しました。

コントローラ

@company.employee_admin_create
# @company.employees.create(name: self.responsible_name,
                          # email: self.email,
                          # password: 'password',
                          # department: '管理者',
                          # joining_date: Date.today,
                          # admin: true,
                          # enrollment_status: true)

モデル

  #企業登録時に、1人管理者権限を持った従業員を登録
  def employee_admin_create
    employees.create(name: self.responsible_name,
                     email: self.email,
                     password: 'password',
                     department: '管理者',
                     joining_date: Date.today,
                     admin: true,
                     enrollment_status: true)
  end

#② if Employee.find_by(email: @company.email)をprivate内に記載
アクション内では1行で記載できるように、privateに記述を引っ越し、
またその際に、ネストが深くならないようにGuard Clauseと呼ばれる技法にて記載。

コントローラ

.
.
check_duplicate_email_in_employees and return
# if Employee.find_by(email: @company.email)
  # @company.errors.add(:email, "はすでに存在します")
  # return render 'new'
# end
.
.

private

def check_duplicate_email_in_employees
  return unless Employee.find_by(email: @company.email)

  @company.errors.add(:email, "はすでに存在します")
  render 'new'
end

#③ そもそも必要の無い記述の削除
下記は、データベース登録時にdefaultでtrueになるカラムであったため、
コントローラでは記述が不要で有ると考え削除

コントローラ

# @company.usage_status = true

④ flashメッセージとredirectを1行にまとめる

コントローラ

redirect_to admin_companies_path, flash: { success: '登録完了しました。' }
# flash[:success] = '登録完了しました。'
# redirect_to admin_companies_path

リファクタリング後のコード

コントローラ

  def create
    @company = Company.new(company_params)
    check_duplicate_email_in_employees and return
    if @company.save
      @company.employee_admin_create
      redirect_to admin_companies_path, flash: { success: '登録完了しました。' }
    else
      render 'new'
    end
  end

  private

  def check_duplicate_email_in_employees
    return unless Employee.find_by(email: @company.email)

    @company.errors.add(:email, "はすでに存在します")
    render 'new'
  end

モデル

  def employee_admin_create
    employees.create(name: self.responsible_name,
                     email: self.email,
                     password: 'password',
                     department: '管理者',
                     joining_date: Date.today,
                     admin: true,
                     enrollment_status: true)
  end

#最後に
最初は自分でも醜いコードだなと思っていましたが、少し意識するだけで、見違えるほど綺麗になりました。
また、リファクタリングは家の中を掃除しているような感覚で、綺麗なコードになると爽快感があるため、
今後も散らかした部屋を一つ一つ掃除していきます。