Why not login to Qiita and try out its useful features?

We'll deliver articles that match you.

You can read useful information later.

3
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?

慣れないデザインパターンを導入して失敗した話

Posted at

概要

  • 筆者が興味本位で使いたくなったデザインパターンを稼働中のプロダクトに導入してみた
  • 導入して約1年後、当該プログラムは存在しているが、同じデザインパターンが他のチームメンバーに使われることがなく、プロダクトコードの中でひとつだけ浮いた状態になった
  • 振り返ってみると、あまり分かりやすくなくて、書いた本人でさえも書き直したくなってしまった

という話です。

発端

某金融システムのバックエンド開発を行なっています。プロダクトコードはRailsで書かれています。
筆者宛ての仕事として、ユーザーからのリクエストをControllerで受け取り、もろもろバリデーションチェックの後、テーブルAのレコードを更新、テーブルB、テーブルCのレコードをそれぞれ1件づつ追加したい、という要件の実装を担当することになりました。
考えられる実装パターンは以下のようなものでした。

選択肢1: そのままControllerに書く

class HogeController < ApplicationController
  def create
    # もろもろのバリデーションチェック..

    ActiveRecord::Base.transaction do
      # テーブルAのレコードを更新
      # ごにょごにょごにょ...
      
      # テーブルBのレコードを追加
      # ごにょごにょごにょ...
      
      # テーブルCのレコードを追加
      # ごにょごにょごにょ...

    rescue StandardError => e
      # エラー処理
    end
  end
end

これはテーブル1件だけを更新するようなシンプルな場合は良いかもしれませんが、テーブル3件が関わっているので、メンテナンスしづらくなってしまう恐れがあります。

選択肢2: Serviceクラスでテーブルを更新する

controller
class HogeController < ApplicationController
  def create
    # もろもろのバリデーションチェック..
    if HogeService.new(params).call
      # 正常終了時の処理
    else
      # 異常終了時の処理
    end
  end
end
service
class HogeService
  def initialize(params)
    @params = params
  end

  # 正常終了時はtrue, 異常終了時はfalseを返す 
  def call
    ActiveRecord::Base.transaction do
      update_table_a
      add_table_b
      add_table_c
      true
    rescue StandardError => e
      # エラー処理
      false
    end
  end

  private

  def update_table_a
    # テーブルAのレコードを更新
    # ごにょごにょごにょ...
  end

  def add_table_b
    # テーブルBのレコードを追加
    # ごにょごにょごにょ...
  end

  def add_table_c
    # テーブルCのレコードを追加
    # ごにょごにょごにょ...
  end
end

これでも良かったかもしれませんが、そのときの私は気軽にServiceクラスを追加することに抵抗がありました。Serviceクラスを導入する場合に一貫したルールがなく、何でもかんでもServiceクラスというまとめ方をされた場合に、全体として訳が分からなくなってしまうと思ったからです。

選択肢3: Interactorオブジェクトを使う

この件に着手する前、私は以下のページを読んでいました。

この内容が記憶に残っていて、メリットも確かにあると感じ、このページと Gem interactor ページを参考にこの方針で実装を進めました。

Controllerから、Organizerを呼び出し、Organizerから各Interactorを呼び出すという構成になります。
(この記事では、このパターンの詳細については触れません。詳しくは、リンク先の記事をご参照ください!)

controller
class HogeController < ApplicationController
  def create
    # もろもろのバリデーションチェック..

    context = HogeOrganizer.call(params)

    if context.success?
      # 正常終了時の処理
    else
      # 異常終了時の処理
    end
  end
end
Organizer
class HogeOrganizer
  include Interactor::Organizer

  organize UpdateTableAInteractor, AddTableBInteractor, AddTableCInteractor

  around do |organizer|
    ActiveRecord::Base.transaction do
      organizer.call
    end
  end
end
Interactor(1)
class UpdateTableAInteractor
  include Interactor

  delegate :params, to: :context, private: true

  def call
    # テーブルAの更新
    return if table_a.update(hoge: params[:hoge])

    # 失敗時の処理
    context.fail!(message: '...')
  end
end
Interactor(2)
class AddTableBInteractor
  include Interactor

  delegate :params, to: :context, private: true

  def call
    # テーブルBの追加
    return if table_b.create(huga: params[:huga])

    # 失敗時の処理
    context.fail!(message: '...')
  end
end
Interactor(3)
class AddTableCInteractor
  include Interactor

  delegate :params, to: :context, private: true

  def call
    # テーブルCの追加
    return if table_c.create(piyo: params[:piyo])

    # 失敗時の処理
    context.fail!(message: '...')
  end
end

この構成にした場合、

  • 各テーブルの追加・更新を1つのクラスで表すことができる
  • 1つづつのクラスの役割が小さく、簡単になる
  • テストコードが書きやすくなる

というメリットがあります。
(メリットとして認識していましたが、どちらかというと「デザインパターンをただ使ってみたい(何となくカッコいいから!)」といったミーハーな理由があったのも確かです)

Interactorオブジェクトを使ったパターンで実装し、開発メンバーにレビューしていただきました。
コメントで「Serviceクラスを使ったほうがよいかもしれない」という意見もありました。私としては「まだこのパターンがどういう使い勝手になるか分からないが、実験的に使ってみたい」という意見を返し、PRはApproveをいただき、無事マージされました。

反省

PRがマージされてから1年ほど経過しましたが、他のメンバーが同じパターンを使って実装することはありませんでした。というか、書いた私自身がもう1回このパターンを適用させるより、Serviceクラスを使ったほうが分かりやすかったかも? と思うようになってしまいました。

考察

では、どうすればよかったか? を考えてみます。

原因の分析

  • 導入者本人が使い慣れていなかった
    私自身、新しいパターンに興味があって導入しましたが、結果として、本当に読みやすく、メンテナンスしやすくなるかどうかのイメージは出来ていませんでした。

  • いきなり既存のプロダクトコードに導入した
    プロジェクトの忙しさや時期にもよりますが、リファクタリングにがっつり時間をかけることは難しいことが多いです。Interactorオブジェクトを使った方法が、後から振り返るとイケてなかった・・と思っても、直すとしたら仕様が変わるタイミングになりそうです。そしてそういう機会はなかなか訪れないかもしれません。

  • Serviceクラスを使った方法でも何とかなった
    Serviceクラスを使う場合、「何でもかんでもServiceクラスに使えてしまい、無法状態になってしまう」という懸念はたしかにあります。そのデメリットに目をつむり、実装でServiceクラスを採用したとしても、それほど悪くはない実装だったかもしれません。
    実装当時、Serviceクラスを使って動作するコードを書いていなかったのですが、比較対象として、Serviceクラスを使った実装PRも作成し、自分で比較検討してみてもよかったかもしれません。

  • 新しいGemに沿ったルールを覚える必要があった
    通常のrailsによる開発とは別に、このGemを導入することによってOrganizer, Interactor, context という独自の概念をイメージする必要がありました。素のrubyと比べ、脳への負担がひとつ増えることになります。

  • 更新処理に伴って階層がひとつ増えた
    Serviceクラスを使う方法であれば、Controller → Service の2つの階層で済みました。Interactorオブジェクトを使う場合、Controller → Organizer → Interactor と3つの階層になってしまいます。
    階層が増えることにより、脳への負担が1階層分増えます。

こうすればよかったかも?

1. ⚪︎⚪︎パターンというワードに踊らされず、本当にメリットがあるかを見極める

「⚪︎⚪︎パターン」という言葉は、かっこいいし、使ってみたくなります。が、必ずしも既存のプロダクトコードにメリットをもたらすものではないかもしれません。小さい問題を解くために、構成が複雑なパターンを導入する必要はないかもしれません。

2. 新規開発に導入する場合は、複数の実装を試す

新規開発時に新しいパターンを導入する場合は、その新しいパターンをひいき目に見てしまうことを防ぐため、地道で愚直なコードのパターンも書いてみて、それと比較する、というのもひとつの手であると思います。

3. 新規開発には導入せず、既存の読みづらい箇所への適用を考える

「2. 新規開発に導入する場合は、複数の実装を試す」は、時間に余裕がある人向けの意見でしょう。現実的には2パターン実装する時間を取れないことが多いと思います。
新しい機能で導入するよりは、既存の動作中のコードに対して、新しいデザインパターンを試したほうがよかったかもしれません。

4. 自分の中に引き出しを準備しておく

記事を書きながら知ったのですが、Gem Interactor は最近あまりメンテナンスされていなくて、代替のGemとして actor というのもあるようです。
すぐにプロジェクトに適用はできないかもしれませんが、引き出しとしてたくさん知っておくのも良いかもしれません。

5. 練習用にリファクタリングする

良いコード/悪いコードで学ぶ設計入門』(仙場 大也著、技術評論社、2022) の 17.2.3 に「リファクタリングで大幅スキルアップ」という記述があります。
設計スキルを高めるための練習として、既存のプロダクトコードに対し、リリースを目的とせず、あくまでプログラマーの練習用としてブランチを切って修正を進めていくということが書かれています。

新しいデザインパターンの習得用として、このやり方がベストかもしれません。

私自身、個人で開発した家計簿アプリケーションを開発していて、それは自分で作ったものなので、どう修正してもよいのですが、「Controllerからテーブル3個更新する」ような複雑な処理は残念ながらありません。

普段の仕事で触っているプロダクトコードのほうが、より複雑でやり甲斐のある(?)修正ネタが埋もれているかもしれません。

試しに適用させたうえで、「これは確かにメリットがある」と実感できたものに関しては、そのままリファクタリングPRとして他の開発メンバーに見てもらってよいのではないでしょうか。

まとめと蛇足

この記事では、筆者があまり馴染みのないデザインパターンをプロダクトコードに導入してみたけど、ほぼ広まらなかった話についてまとめてみました。

筆者は日々の業務でコードを触りつつ「ここ、汚いなあー」と思うようなプログラムがたくさんあるのですが、全てを修正できるわけではありません。多少汚くても動いています。動いていることは正義です。そこに敬意を持つようにしたいと思います。

「きれいに書くこと」を突き詰めていくと、おそらく時間がめっちゃかかります。
最近は、デザインパターン等は無理に使わず、ごく基本的な知識(命名を適切にする、長すぎるメソッドは書かない、ネストを浅くする、SOLID原則を意識する、等々・・)だけでも、確実にプロダクトコードに反映させることは大変ですし、価値があることだと思います。

「あるコードが読みやすいか、読みづらいか?」という問題も、おそらく突き詰めて考えるとどうしても人の主観が混じるものかと思います。どこかで程よい「割り切り」が必要かもしれない・・というのが最近私の考えていることです。

3
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

Qiita Advent Calendar is held!

Qiita Advent Calendar is an article posting event where you post articles by filling a calendar 🎅

Some calendars come with gifts and some gifts are drawn from all calendars 👀

Please tie the article to your calendar and let's enjoy Christmas together!

3
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?