0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 1 year has passed since last update.

コントローラーのパブリックインターフェースを「どのように」から「何を」へリファクタリングする

Last updated at Posted at 2023-04-02

何をしたか

コントローラーのアクション内の処理に関する 6行 の記述を、
処理内容がわかりやすいメソッド名にし、 1行 にしました。
また、「どのように」から「何を」 を意識するようにリファクタリングしました。

どんなメリットがあるか

  • createアクションの可読性が上がる

具体的な内容

ApplyController#createでの処理です。
(Applyが単数系なのは、このコントローラがAPIからのリクエストを受け取るコントローラであり、
indexなどの複数のリソースを扱わないためです。)

Applyという応募情報を管理するテーブルがあり、createアクションは応募情報を作成する処理です。
Applyには下記のカラムが存在します。(便宜上、必要最低限のカラムのみとしています。)

  • メールアドレス...email
  • 生年月日...birthday
  • A社のアフィリエイトから訪問した際にパラメータに含まれるID...a_id
  • B社のアフィリエイトから訪問した際にパラメータに含まれるID...b_id

作成処理(createアクション)では、
アフィリエイト経由の場合、
経由もとアフィリエイトへ、成果HTTPリクエストを送りたいという仕様でした。

また、アフィリエイトは2社利用されており、それぞれの送信の条件が異なりました。

a_idはA社へ、
b_idはB社へ送信します。

送信条件としては、
どちらも開発環境では送信させたくないため、環境での条件が科せられています。

A社に関しては、すでにUserという会員情報を管理するテーブルに、
同じメールアドレスでレコードが存在する場合は送信させたくないようです。

また、A社は20~50代の応募のみを対象としています。

class ApplyController < ApplicationController

  def create
    # 受け取ったパラメータでインスタンス化
    apply = Apply.new(apply_params)
    # Userという会員情報を管理するテーブルにすでに存在するメールアドレスの応募か確認している
    is_exists_user_email = User.exists?(email: apply.email)

    # 処理が続く...

    if apply.a_id.present? && !is_exists_user_email && apply.include_age?(range: 20..59)
        && (Rails.env.production? || Rails.env.pre?)
      Net::HTTP.get_print(URI.parse("https://a-company?a_id=#{apply.a_id}"))
    end

    if apply.b_id.present? && (Rails.env.production? || Rails.env.pre?)
      Net::HTTP.get_print(URI.parse("https://b-company?b_id=#{apply.b_id}"))
    end

    # 処理が続く...
  end

end

A社へ送信するリクエストの条件に利用しているApply#include_age?
Applyのbirthdayカラムから年齢を算出し、Apply#include_age?rangeキーワード引数に渡された範囲の年齢かを真偽値で返すメソッドです。

class Apply
  # 生年月日から年齢を返すメソッド
  def age_calculation; end

  # 年齢がRangeオブジェクトの範囲内かを真偽値で返すメソッド
  def include_age?(range:)
    range.include?(age_calculation)
  end
end

これらを踏まえてやりたかったこと

  • createアクションの可読性を上げる
  • 「どのように」から「何を」を意識した設計にする

どのようにしたか

createアクション内では

  • どんな条件で
  • どこへ
  • 何をするか

が全て指示されているような設計となっていました。
これは参考文献の4章でも取り上げられていた

犬を散歩へ連れ出すことを考える。
犬のに直接「歩け」と命じるのはおかしい。
この場合、犬に対して命令し、自分の足の面倒は自分で見させるのが正しい方法。

という考え方を適用してリファクタリングすることにしました。

具体的には 「アフィリエイトへ報酬リクエストを送信する」 という
send_affiliateプライベートメソッドとしました。

またsend_affiliateでは、A社とB社で条件が重複している箇所は一箇所にまとめ、
早期リターンを活用することで、コードを全て読まなくても良いようになりました。

class HogeController < ApplicationController

  def create
    apply = Apply.new(apply_params)
    is_exists_user_email = User.exists?(email: apply.email)

    # 処理が続く...

    send_affiliate(apply, is_exists_user_email)

    # 処理が続く...
  end

  private

  def send_affiliate(apply, is_exists_user_email)
    return unless Rails.env.production? || Rails.env.pre?

    if apply.xuid.present? && !is_exists_user_email && apply.include_age?(range: 20..59)
      Net::HTTP.get_print(URI.parse("https://a-company?a_id=#{apply.a_id}"))
    end

    return if apply.cid.blank?

    Net::HTTP.get_print(URI.parse("https://b-company?b_id=#{apply.b_id}"))
  end
end

これにより、createアクション内のコードリーディング時には、
send_affiliateという単語から分かるように
「アフィリエイトへリクエストを送信しているんだな」ということがわかりやすくなりました。

今後

今回はパブリックインターフェースを単純にプライベートへ移しただけなのですが、
「そもそもHTTPリクエストなどを送るメソッドをコントローラーのプライベートメソッドに記述するのが最適なのか?」
といった疑問があがりました。

「モデルのメソッドとして管理する方法はどうなのか?」
「アフィリエイト専用のクラスを用意するのがいいのか?」
といった、方法も考えつきました。

これらをどのように判断するべきか、判断力がまだ備わっていないことを痛感したため、
参考文献を繰り返し読み込み、判断力を養っていこうと思います。

また、何かベストプラクティスがある場合ご教授いただけると幸いです。

参考文献

0
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?