STORES 予約 で Webアプリケーションエンジニアをしています。ykpythemindです。
STORES 予約 では機能開発する際に積極的にリファクタリングを行っています。 前回 は支払い方法API部分のリファクタリングを途中まで紹介しました。後編ではより踏み込んだリファクタリングを実施し、凝集度の低いクラスを削除します。
内部実装を知らなくて済むように分岐を減らす
前回、予約確定後に使った支払い方法を記録する部分には問題がありました。
# 予約に対して支払いを実施し、結果を記録します def update_payment!(reservation, payment_request) payment_service = Reservation::PaymentService.new reservation.merchant # 支払い方法によって分岐 case payment_request.type when :subscription subscription = current_user!.end_user_customer_subscriptions.find_by( public_id: payment_request.customer_subscription_id, ) payment_service.pay_with_subscription!(reservation, subscription) when :ticket product = Product.find_by(public_id: payment_request.product_id) ticket_book_ids = current_user!.end_user_ticket_books.where(product: product).available.pluck(:id) payment_service.pay_with_tickets!(reservation, current_user!.end_user_tickets.valid.where(ticket_book_id: ticket_book_ids) when :credit_card payment_service.pay_with_credit_card!(reservation, remote_ip, payment_request.ctok, credit_card) end end
パラメータとして予約者から与えられた支払い方法が実際に正しいのか(月謝の期限が切れていないか、不正なパラメータが与えられていないかどうかなど)の検証するクラスでも、同様の分岐が発生していました。
class PaymentMethodValidator def initialize(reservation, payment_request:, user:) @reservation = reservation @payment_request = payment_request @user = user end def validate_payment_method case @payment_request.type when :subscription return validate_payment_method_subscription(@reservation, payment_request.customer_subscription_id) when :ticket return validate_payment_method_ticket(@reservation, payment_request.product_id) when :credit_card return validate_payment_method_credit_card(@reservation) end return { success: true } end def validate_payment_method_credit_card(reservation) # クレジットカード支払いを使用できるかのバリデーション... end # ...
支払い方法の種類によってどのようなメソッドを呼ばなければならないのかを常に実装者が把握していなければならないという点や、PaymentService / PaymentMethodValidator class自体の凝集度が低いという点で好ましくありません。
例えば今回実装したい機能である「クーポン支払い」を実装する際に、PaymentRequestのtypeによっての分岐を漏れなく書く必要がありました。
ここではリファクタリング第2版『ポリモーフィズムによる条件記述の置き換え』を行い、分岐を削除しましょう。*1
ポリモーフィックに振る舞うため、PaymentRequestに実装するインターフェースを決めましょう。コードを読むと支払い方法を検証する部分(PaymentMethodValidator)と支払いを実施する部分(PaymentService) の2つのクラスがあり、どちらも予約を表すmodel Reservationを受け取っています。したがって以下のメソッドを各PaymentRequest側に実装します。
#pay(reservation: Reservation, current_user: User | nil) -> boolean
#validate(reservation: Reservation, current_user: User | nil) -> PaymentRequest::ValidationResult
新しい支払い方法が追加されたときは、この2つのメソッドを実装したPaymentRequestを実装すれば良くなり、使用者は内部実装を知らずにメソッドを呼び出せばいいだけになります。
class PaymentRequest # ValidationResultは支払い方法のバリデーションの結果を保持するクラスです。 ValidationResult = Struct.new(:message, :success, keyword_init: true) do def valid? success end def invalid? !valid? end end class Base # 結果オブジェクトを生成するヘルパーメソッドです。お好みで定義してください。 def validate_error(message) ValidationResult.new(message: message, success: false) end def validate_success ValidationResult.new(success: true) end end # (前記事参照) クレジットカード払い class CreditCard < Base # ... def validate(reservation:, current_user:) # 注釈) 該当の予約に紐づく予約ページの設定で、実際にクレジットカード払いを要求しているかを調べる unless valid_method_for_reservation?(reservation) return validate_error('not_supported') end # ... 中略 ... # (ここの内容は PaymentMethodValidatorのクレジットカード払いの場合に書かれていた検証を持ってくる。) validate_success end def pay(reservation:, current_user:) # (ここの内容は PaymentService#pay_with_credit_card! を呼んでいる部分を持ってくる ) end end end
PaymentMethodValidatorには validate_payment_method_ticket / validate_payment_method_credit_card のような検証用メソッドがありましたが、それをすべて validateインターフェースの実装として各PaymentRequestに移植しましょう。
支払いを行わない場合も、なにもしない pay メソッドを定義するようにします。
# (前記事参照) 支払いを行わない場合 class Noop < Base # ... def validate(reservation:, current_user:) # 注釈) 該当の予約に紐づく予約ページの設定で、支払いを行わなくても良いかどうか確認 unless valid_method_for_reservation?(reservation) return validate_error('not_supported') end validate_success end def pay(reservation:, current_user:) # なにもしないで成功扱い true end end end
支払い方法ごとのPaymentRequestにpay/validateメソッドを定義できたら、あとは使用箇所を置き換えます。
前回の記事で初めに載せたリクエストの処理部分を変更しています。
diff --git a/r3.rb b/r2.rb index fb70541..2752d11 100644 --- a/r3.rb +++ b/r2.rb @@ -5,17 +5,11 @@ post '/payment/validate' do # 特定の支払い方法の場合はログインが必須 user = current_user! if payment_request.need_user_login? - # すべての処理をValidation classに委譲 - payment_method_validator = - ::Reservations::PaymentMethodValidator.new( - @reservation, - payment_request: payment_request, - user: user, - ) + validation_result = payment_request.validate(reservation: @reservation, current_user: user) # 選択した支払い方法が正しくなければ構造化したエラーレスポンスを返す。 - unless payment_method_validator.valid? - error!(payment_method_validator.payment_method_error, 400) + if validation_result.invalid? + error!(validation_result.message, 400) end status :ok @@ -45,23 +39,17 @@ post '/finalize' do user = current_user! if payment_request.need_user_login? # このエンドポイントでも同様に支払い方法パラメータを検証する - payment_method_validator = - ::Reservations::PaymentMethodValidator.new( - @reservation, - payment_request: payment_request, - user: user, - ) - - unless payment_method_validator.valid? - error!(payment_method_validator.payment_method_error, 400) + validation_result = payment_request.validate(reservation: @reservation, current_user: user) + + # 選択した支払い方法が正しくなければ構造化したエラーレスポンスを返す。 + if validation_result.invalid? + error!(validation_result.message, 400) end # -- 中略 -- - # 支払い方法が設定されているページでは支払いを実施し、レコードを作成する - if @booking_page.require_payment? - update_payment! @reservation, payment_request - end + # 支払いを実施し、レコードを作成する + payment_request.pay(reservation: @reservation, current_user: user) end
置き換えができたら、 既存の PaymentMethodValidator やupdate_payment!メソッドを削除できます。
最後に、シーケンス図のbefore/afterを掲載しておきます。
before
after
エラーレスポンスを返す箇所も少なくなり、使われるクラスも少なくなって実装としてシンプルになりました。
まとめ
いかがでしたでしょうか。
実際3回ほどのPull Requestに分けて改修を実施しましたが、だいたい2日ほどですべての作業が終わっており、すぐにリターンが得られそうです。*2
エンジニア間でも「もし決済手段にコンビニ決済を追加したかったら新しいPaymentRequestを実装すればいいね」みたいな会話ができるようになりました。
STORES 予約 では日々機能の開発しながら、メンテナンス性も重要視しています。10年、20年先も使われるプロダクトでありたいと思っています。