実録!!データ構造リファクタリング -- 僕とメッセージ機能の300日戦争

みなさんもきっとそうだと確信いたしておりますが、プログラマというのは、どういうわけか実装のちょろまかしには頭がまわるもので、今や丁寧なコードを書く人の鏡とまで言われるワタクシも、それはそれは手抜き方法ばかりうかんだものでした。

技術的投資のいくつかは、不本意ながら技術的負債になりまして、いろいろと世間様にもご迷惑をおかけした次第です。みなさんもきっとそうだと思いますが。
この話は、そんな「誰にでもある」小さな事件のひとつです。[^1]

[^1]: [ぼくたちと駐在さんの700日戦争](http://700days.blog69.fc2.com/blog-entry-2.html) の書き出しより。

----

この記事は [CrowdWorks Advent Calendar](http://qiita.com/advent-calendar/2016/crowdworks) 21 日目の記事です。
昨日は @tmknom さんの [「アプリケーションアーキテクチャに関するポエム」](http://qiita.com/tmknom/items/be5c4b350f561991f2f5) でした。
設計に関するトピックは幅広く、かなり広範な知識が求められますよね！早く DDD を読まねばという気分になりました（笑）。

さて、この記事は、著者がここ１年ほど携わった簡単なデータ構造の改修に対する、個人的な回顧録です。技術的な記事というよりも、どちらかというと **ポエム** のような何かです。いくつか、１年間の失敗点・反省点をまとめたメモを付けておきました。もしも同じような作業をする際には参考にしていただければ幸いです。


なお、この記事は*半フィクション*です。

あと、大事なことなので太字にしておきますが **銀の弾丸なんてありません**

----


## この素晴らしい実装に祝福を！

2016年1月。新年も明けて、なんだか新しい気分で働き始めたある日、ボスからメッセージまわりのリファクタリングを、取り組んでよいとの許可が出ました。

「先輩！やりましたね！これでやっと `MessageVirtualThread` クラスを何とかできますよ！」
「あの複雑怪奇なクエリも、見通しよくできるなぁ」
「 **仕事を依頼していないと絶対にメッセージが送れないデータ構造を直しましょう！？** 」

一見すると意味不明なコメントですが、これには深いワケがあるのです……

その昔、クラウドワークスのメッセージ機能は、とても単純で、あるユーザが別のユーザにメッセージ、つまりなんかの文字列を送れるだけの機能でした。ですので、最初の設計者は、とてもシンプルに実装したのです。

![cd01.png](https://qiita-image-store.s3.amazonaws.com/0/53253/ec7420fd-1936-7a4c-5657-e8aacef4c902.png)

１通のメッセージを表す `Message` とその中間テーブルである `UserMessage` の２つのモデル。そして、何に紐づいたメッセージなのかを表すポリモーフィック関連。とてもシンプルです。そして、歴史はここから積み上がっていきました……

最初の実装から**３年半**。負荷対策や新機能への対応など、様々なコードの修正を経て、データ構造はこんな感じになっていました。（注 **イメージです**）

![od02.png](https://qiita-image-store.s3.amazonaws.com/0/53253/a9323d82-456b-cad0-6db3-7fa65f13df57.png)

「先輩！いったい何でまたこんな感じになっちゃったんですか〜？？　だいたい *virtual_thread_id* ってなんですか！？」
「……いや、仕方なかったんだよ」
「うーん。どこが仕方なかったんですか〜」
「いやさ、今でこそスレッド一覧とスレッド詳細みたいな画面構成だろ？」
「はい。どう見てもスレッドの中にメッセージがありますよ！」
「いや、初期設計は *電子メール* っぽい仕様を考えてたんだよ」

はい。当時のメッセージは基本的に独立しており、どちらかというとメールクライアントのような UX を想定していました。

「で、UXの改善をしているうちにどんどんスレッドじみてきてな」
「何でその時スレッドモデルを作らなかったんですかぁ〜」
「ウッ……その時作ったのが `virtual_thread_id` でさ、最初は負荷対策として `messages` に付け足してみたんだ」
「えー。なんでその時、`virtual_thread_id` を変な文字列にしたんですか〜？？」
「いやほら。 あくまでも"仮想"のスレッドだろ？」
「えー？」
「ちょうど `"#{user_id}_#{user_id}_#{job_offer_id}"` 形式が使いやすくてな」
「えー…」
「Message モデル、もう `thread_id` ってのあったし…… **つい、な**」
「えー！？」
「で、そのあと、`MessageVirtualThread` モデル作ったんだ」
「何でそんなことしちゃったんですか〜！！」

いやはや、 **その時々の負荷対策や工数に見合った最適化の末**、大変なことになったものです。

「まぁ、**技術的投資**ってやつだよ」
「**負債になりかかって**ますけどね〜」

> メモ
> ３年も運用すればこんなもん
> まさかUIがこう変わるとは……

## この実装には問題がある！

「ふむ……で、何をどうやって直していくんだい？」
「はい、まずは何が問題なのか整理しようと思います」
「そうだね。問題の整理は大事だね」

当たり前ですが、そこそこ大きなリファクタリングをする場合には、あらかじめ「どこを」「どの程度」「どう直すか」決めておいたほうが良いものです。
そもそも何がどう問題なのかを整理せず、場当たりてきにリファクタリングをすると、 **より悲惨なことに** なる場合がありますし。

「問題点は大きく言って次の通りだと思います」

* UIがスレッドベースなのに、データ取得や処理が `UserMessage` をベースとしているのにはそろそろ無理がある。
* データ構造上、1to1メッセージにしか対応してないし、何か仕事と関連していないとメッセージが送れないのはユーザビリティ的に問題がある
 * `MessageVirtualThread` の `id` は `"#{user_id}_#{user_id}_#{job_offer_id}"` という String 型
 * `UserMessages` には `sender_id` と `receiver_id` というカラムがある

「なるほど。この２つを解消すれば、グループメッセージ機能やその他メッセージの改修に入れる、と。」
「はい。あとは、妙な依存をなるべく消していければと考えています」
「ふむ。どういうデータ構造にするつもりだい？」

チームですったもんだの議論の末に出来上がった再設計案が次の通りです。

![od03.png](https://qiita-image-store.s3.amazonaws.com/0/53253/587901df-64ba-8ea7-ca9b-e41b74a6d917.png)

「いいんじゃないかな。さて、どうやって理想に近づけていくか」
「いきなり全とっかえってのはどうでしょう？」
「……本気で言ってるのかな？」

これも当たり前のことですが、明らかにコードの変更量などが大きくなりそうな場合、可能なかぎり **細かく分割してリリース** すべきです。
運用中のコードを大きく変更しすぎると、メインストリームのコードと乖離が大きくなりすぎるケースがよくあります。
今回のようなリファクタリングでは、少しづつ本番に修正を適用していったほうがリスクが少なく運用できます。

「まずは `MessageVituralThread` の主キー変更からやろうと思います」
「確かに、一番リスクがありそうな場所だし、良いんじゃないかな」
「次に新しいデータ構造を追加しつつ、最後にアプリケーションコードの修正をするつもりです」
「良さそうだね。それでどれくらい時間がかかりそうなんだい？」
「そうですね……最短で**２−３ヶ月というところでしょうか**」
「そうか……（これは半年以上かかるな）」

> メモ
> 自信がない場合は、自分の見積もりの３倍の時間をボスに言いましょう

## NEW MESSAGE!

「先輩。ちょっといじり始めようと思ってソース読み始めたんですけど、そもそもこの初期化時のパラメタはどうなってるですか？？」
「げ……どの辺が疑問？？」
「いやですね、１人分の `user_message` だけ作ろうとしているじゃないですか。でも、結果的に *`user_message` って２人分もできます* よね？」
「あー……そこは、ほら、コールバックでさ……」

```ruby:before
message = proposal.messages.build(
  body: 'hoge',
  user_messages_attributes: [
    {user_id: params[:receiver_user_id] }
  ]
)
message.user = current_user
message.save
```

一見すると不思議なパラメタですが、まぁ慣れると簡単です。
メッセージ作成時に、受信者は `message.user_messages_attributes` で指定、送信者は `message.user` で指定するだけです。
すると、コールバックのマジックによっていい感じにデータができます。
**わかれば簡単ですよね？？本当にゴメンなさい**

「**えっ……この間一括処理スクリプト書いた時、逆にしちゃいましたけど……**」
「えっ」
「もー！　どうしてそういうコードにしたんですかー！！」

わかりにくいパラメタ指定はたまに悲劇を招くものです。
作った本人にはわかりやすかったのですが……

「先輩！どうなおします？」
「うーん、とりあえず、`user_message` は少なくともメッセージ作成時には隠蔽すればいいかな？」
「なるほど！とりあえず、 `message.receiver` とかにしますか！ で、何箇所くらいあるんですかね？」

嗚呼、 grep するだけで憂鬱な気分に。インターフェースの変更はコストがかさみます。

```ruby:after
message = proposal.messages.build(
  body: 'hoge',
  receiver: User.find(params[:receiver_user_id])
)
message.user = current_user
message.save
```

全部まとめて修正していくわけにもいかず、 **新旧のパラメタを両方許容修正を入れてから、一箇所ずつ順番に直す羽目に**なるのでした。

## キーの名は。

「さて、新しいテーブル作る前に、スレッドIDの形式変更だね」
「primary key の変更か……安全……なんですかね……？」

```ruby
class ChangePrimaryKeyToMessageVirtualThread < ActiveRecord::Migration
  def up
    execute "ALTER TABLE message_virtual_threads DROP PRIMARY KEY;"
    execute "ALTER TABLE message_virtual_threads ADD COLUMN `new_id` INT(11) AUTO_INCREMENT FIRST, ADD PRIMARY KEY (new_id);"
    add_index :message_virtual_threads, :id
  end

  def down
    remove_index :message_virtual_threads, :id
    execute "ALTER TABLE message_virtual_threads DROP PRIMARY KEY, DROP COLUMN new_id;"
    execute "ALTER TABLE message_virtual_threads ADD PRIMARY KEY (id);"
  end
end
```

「うーん。`new_id` とか、きっもち悪いなぁ・・・」
「先輩！これ、Railsのコードきちんと動くんですか〜？」
「いや、俺も自信なくてさ……結局こうした」

```ruby
class MessageVirtualThread < ActiveRecord::Base
  self.primary_key = :id
end
```

「・・・なんというか、すごいっすね」
「まぁ、な」

> メモ
> 結論、これで大丈夫でした

「ついでに `messages` と `user_messages` にも新しい id 記録するカラム追加しないとな」

```ruby
class AddNewIdToMessageAndUserMessage < ActiveRecord::Migration
  def change
    add_column :messages, :new_thread_id, :integer
    add_column :user_messages, :new_thread_id, :integer
  end
end
```

「こっちはカラム足すだけだから簡単ですね〜」
「さ、さっさとメンテして、適用しちゃうか」

結論から言えばこのメンテナンスは２回、失敗しました。
1回目はテーブルのコピー中に**ディスクが枯渇し**、ディスクを増強した2回目はコピーが終わらずメンテナス**予定時間を大幅に超過しそうなので打ち切り**になってしまったのです。

> メモ
> MySQL では ADD COLUMN する際に一時テーブルを作成します。サイズの大きなテーブルでは注意！事前に時間を計測しておくとベターです

「うわあああああ」
「落ち着いてください！先輩！」
「ううっ……代替案はあることはあるけどさ……」
「どうするんです？？」
「`ALTER` が終わらないなら、 `INSERT` と `SELECT` を組み合わせて新スキーマにデータ移動した後、 `RENAME table` するんだよ」

テーブルサイズによっては `ALTER TABLE tbl_name ADD colmun` するよりも、新スキーマの一時テーブルを自作し、 `INSERT INTO tmp (col1, col2) SELECT col1, col2 FROM old_table` みたいにした方が早いケースがあります。
今回の例ですと、 `ALTER TABLE` は４時間、 `INSERT INTO` はデータ移行が 1時間20分で、 `rename table` が 10 分以内に終わりました

「**バッドノウハウ感**がすごいですね。先輩」
「言うな……」
「でもカラムの追加はできましたね！後はデータの移行しましょう！」

データ移行の注意点は次の３つ。

1. まずはソースコードを修正して、新しいレコード保存時に、必要なカラムが埋まるようにすること
2. 次に、SQLを流すなりスクリプト書いてバッチ処理するなりで既存のレコードのカラムを埋めること
3. 最後に、 `NOT NULL` 制約をかけるなり、Rails のバリデーションを追加するなりして、データの整合性を強固にすること

「ソースコードはどうすればいいですか？」
「あんまりコールバック増やすのは嫌だけど、こういう時こそ**Rails のコールバックは便利**なんだよね」

「データの移行はバッチですか？」
「今回はこんな感じの SQL 書いて対応するよ」

```sql
UPDATE messages
JOIN message_virtual_threads
ON messages.virtual_thread_id = message_virtual_threads.id
SET message.new_thead_id = messages_virtual_threads.new_thread_id
```

「あとは制約かけておしまいですね！先輩！」
「そうだね……メンテナンス多いなぁ……」

新規のモデル・テーブル追加は至って簡単だったので割愛。既存データの作成はバッチ処理でやりました。**バッチ処理に１週間くらいかかりましたが**……

```ruby:script/onetime/20160701_setup_user_thread.rb
MessageVirtualThread.find_each do |virtual_thread|
  ActiveRecord::Base.transaction do
    if virtual_thread.id =~ /\d+_\d+_\d+/
      virtual_thread
      virtual_thread.touch_content!
      user_ids, _ = parse_id(virtual_thread.id)
      user_ids.each do |user_id|
        ut = Message::UserThread.where(user_id: user_id, message_thread_id: virtual_thread.id).first_or_initialize
        ut.starred_flag = starred_flag(user_id, virtual_thread.id)
        ut.latest_read_message_id = latest_read_message_id(user_id, virtual_thread.id)
        ut.touch_content!
      end
      puts "CONVERTED\t#{virtual_thread.id}\t#{virtual_thread.created_at}"
    else
      puts "INVALID\t#{virtual_thread.id}\t#{virtual_thread.created_at}"
    end
  end
end

slack_post "#psychic_research_group", "バッチ処理終わったよー"
```

## スレの形

データ構造がいい感じにできれば、参照系のリファクタリングに着手できます。

「よし、"スレッド一覧画面" と "スレッド詳細画面" を改修しよう」
「最近、 **DBのCPUアラートも多いですしね〜**」
「……**ゴメンな**」

既存のコードはこんな感じ。（イメージです）
`UserMessage` を主体としてデータ取得をしようとし、**数々の負荷対策**により何かすごいコードになっています。

```ruby:app/controller/messages_controller.rb
class MessagesController < ApplicationController
  def index
    ids = UserMessage.
      select('MAX(user_messages.id) AS id').
      group('user_messages.virtual_thread_id').
      merge(UserMessage.where(user_id: current_user.id))
    @threads = UserMessage.
      joins("INNER JOIN (#{ids.to_sql}) ids ON user_messages.id = ids.id")
  end

  def show
    @messages = UserMessage.
      where(user_id: current_user.id).
      where(virtual_thread_id: params[:virtual_thread_id]).
      joins('JOIN messages ON messages.id = user_messages.message_id')
  end
end
```

「まぁ、あんまり迷わないよね」

```ruby:app/controller/messages_controller.rb
class User < ActiveRecord::Base
  has_many :user_threads, class_name: 'Message::UserThread'
  has_many :threads, class_name: 'MessageVirtualThread', through: :user_threads
end

class MessagesController < ApplicationController
  def index
    @threads = current_user.threads
  end

  def show
    @messages = current_user.threads.
      find_by(params[:virtual_thread_id]).messages
  end
end
```

「これで完成……あれ！？動かないぞ！？」
「やだなぁ、先輩。よくみてくださいよ。元コードの `@threads` は `UserMessage` の配列ですよ？？」
「**まじかよ**」

## クオリティア・コード

データ構造は出来上がりました。
こうしている間にも、新しいスキーマのデータがどんどんできていきます。
あとは、新しいデータ構造に対応するように、Rails のコードを治していく**だけ**です。

「進捗はどうだい？」
「はい。ここまでは順調です。見積もり通りあと１ヶ月ほどで終わるかと」
「ふむ。それは重畳。だけどコードの修正は結構時間かかるんじゃないかな？」
「はぁ。まぁ、やってみます」


「さて、改めてコードを見てみよう」
「先輩！結構、コールバック複雑じゃないですか？」
「**……ゴメンな**」

```ruby:app/models/message.rb
class Message < ActiveRecord::Base
  has_many :user_messages

  belongs_to :thread, class_name: 'Message', foreign_key: 'thread_id'

  before_validation :set_transceiver_on_receiver_user_messages

  before_create :set_job_offer_id,
    :exclude_sender_from_recipients,  
    :set_virtual_thread_id_to_user_messages

  after_create :create_sent_user_message,  
    :set_thread,
    :update_thread,
    :create_or_touch_proposal,
    :create_virtual_thread

  def set_transceiver_on_receiver_user_messages
    user_messages.each do |user_message|
      user_message.sender_id   = user_id
      user_message.receiver_id = user_message.user_id
    end
  end

  def set_job_offer_id
    return if job_offer_id.present?

    if messageable.class.to_s == 'JobOffer'
      messageable.id
    elsif messageable.present? && messageable.respond_to?(:job_offer)
      messageable.job_offer.id
    elsif thread.try(:job_offer_id)
      thread.job_offer_id
    end
  end

  def exclude_sender_from_recipients
    user_messages.each do |user_message|
      user_messages.delete(user_message) if user_message.user == user
    end
  end

  def set_virtual_thread_id_to_user_messages
    self.virtual_thread_id = MessageVirtualThread.generate_virtual_thread_id(
      [user_messages.map(&:user_id), user_id], job_offer_id
    )
    user_messages.each do |user_message|
      user_message.virtual_thread_id = virtual_thread_id
    end
  end

  def create_sent_user_message
    user_message = user_messages.first
    user_messages.create(
      user: user,
      sender: user,
      receiver: user_message.receiver,
      virtual_thread_id: user_message.virtual_thread_id,
      read_flag: true,
    )
  end

  def set_thread
    update_attribute(:thread_id, id) if thread_id == 0
  end

  def update_thread
    unless thread?
      thread.update_attribute(:updated_at, DateTime.now)
    end
  end

  def create_or_touch_proposal
    proposal = case messageable_type
      when 'Proposal'
        messageable
      when 'Proposal::Foo', 'Proposal::Bar', 'Proposal::Baz'
        messageable_type.constantize.unscoped do
          messageable.proposal
        end
      else
        Proposal.where(job_offer_id: job_offer.id, user_id: user_ids).first if job_offer.present?
      end
    if proposal.blank?
      if job_offer.present?
        _proposal = job_offer.proposals.build
        # 以下 `_proposal` をごにょごにょする
        unless _proposal.save
          logger.error("failed to save the proposal. #{_proposal.inspect}")
          raise ActiveRecord::Rollback
        end
      end
    else
      if proposal.some_cond?
        proposal.update_column(:status, Proposal::SOME_STATUS)
      end

      if !proposal.other_cond?
        if !proposal.replied? && user_id == proposal.receiver.id
          proposal.reply!
        else
          proposal.touch
        end
      end
    end
  end

  def create_virtual_thread
    virtual_thread = MessageVirtualThread.where(id: virtual_thread_id).first_or_create(proposal_id: proposal.id)
    virtual_thread.touch_content
  end
end
```

「先輩！いったい何でまたこんな感じになっちゃったんですか〜？？　だいたい *create_or_touch_proposal* ってなんですか！？」
「……いや、仕方なかったんだよ」
「うーん。どこが仕方なかったんですか〜」
「いやさ、応募に "気軽な相談" ってステータスあるだろ？」
「はい。確かにありますね。」
「あれ、実装するときにさ、メッセージ送ったら応募ができるようにしようって思って……」
「うーん……**ダメですよ！**」

言い訳がてら、**どうしてこうなったのか**を説明すると、だいたい次のループで構成されます。

1. 負荷対策とかで、とりあえず他データ参照用のカラムを追加する
2. そのカラムが便利だったので内部的に依存するコードを追加する
3. 依存がきつく、`nil`だと不便なのでカラムを埋めるためのコールバックを用意する

そうして、**依存関係がガッチリしたコールバック群** が生まれることになったのです。

「まずは落ち着いて依存関係を整理しよう」
「複雑ですねぇ」
「まぁ、肝になるのは `job_offer`, `thread_id`, `proposal` のあたりがどういう順番で依存しているかだよ」

「まず、 `job_offer_id` をセットするためには `thread` が確定している必要がある」

```ruby
  def set_job_offer_id
    return if job_offer_id.present?

    if messageable.class.to_s == 'JobOffer'
      messageable.id
    elsif messageable.present? && messageable.respond_to?(:job_offer)
      messageable.job_offer.id
    elsif thread.try(:job_offer_id)
      thread.job_offer_id
    end
  end
```

「そして、 `proposal` を作る前に、 `job_offer` が決まっている必要がありそうだ」

```ruby
    proposal = case messageable_type
      when 'Proposal'
        messageable
      when 'Proposal::Foo', 'Proposal::Bar', 'Proposal::Baz'
        messageable_type.constantize.unscoped do
          messageable.proposal
        end
      else
        Proposal.where(job_offer_id: job_offer.id, user_id: user_ids).first if job_offer.present?
      end
```

「ついでに言えば、 `MessageVirtualThread` を作るためには `job_offer` が確定されている必要がある」

```ruby
  def set_virtual_thread_id_to_user_messages
    self.virtual_thread_id = MessageVirtualThread.generate_virtual_thread_id(
      [user_messages.map(&:user_id), user_id], job_offer_id
    )
    user_messages.each do |user_message|
      user_message.virtual_thread_id = virtual_thread_id
    end
  end
```

「つまり、最初に殺すのは `create_or_touch_proposal` だ！」
「先輩、**一番大変そうなやつ**からになりましたね……」
「……これ、**１ヶ月じゃ終わらない気がしてきた**」

スケジュールがとても怪しくなってきました。気を取り直してコードの読解を続けましょう。

「どういうときに `proposal` が `blank` になるんだ？？」
「えーっと、 `messageable` が `Proposal`, `Proposal::Foo`, `Proposal::Bar`, `Proposal::Baz` じゃなくて、その上で `Proposal.where(job_offer_id: job_offer.id, user_id: user_ids)` が `blank` の時ですね」
「う……で、`job_offer.id` が `nil` になるケースは……」
「`set_job_offer_id` 見てくださいよ！先輩！」

コールバックと条件判定が絡み合ってそこそこ複雑です

「柔軟すぎる設計も、考えものだなぁ」
「どうします〜？」
「これだけ絡み合ってるなら、もう一気に書き換えるしかないんじゃないかな……ちょっとボスと相談させてくれ」
「うーん。**一気にやるのはリスキーですよ**。まぁ、相談から帰るのお待ちしてますね」

## Re:ゼロから始める再実装

「なんだい？　相談があるんだって？」
「はい、ちょっとコードの改修に手間取ってまして……どうも、きちんと安全にやるには一斉にリリースをせざるをえない気がしまして」
「ふむ。ちょっと理想的にやろうとしすぎているんじゃないかな？　理想的な方法が取れないなら、現実的な方法として１歩ずつ進むしかないよ。まず、どこが課題なんだい？」

迷った時ほど、一時停止して、 **どこでどう詰まっているのか** をよく考えてみるべきでしょう。
今回のケースでは、一見すると「コールバックとその条件が複雑すぎる」という点で詰まっているように見えます。
でも、データ構造と照らし合わせてよく考えてみると **Message 作成時に、必ず作成済みの MessageVirtualThread を指定するように修正すれば** 万事解決します。

「つまり、整理するとこんな流れになる」

* Proposal has one MessageVirutalThread なのだから、Proposal のコールバックでスレッドを作る
* メッセージ作成時には、その MessageVirtualThread の id を渡すようにする
* メッセージ内での `proposal` や `job_offer`, そして `users` の参照はスレッドに移譲する

「すると、大半のコールバックは消せるんじゃないかな？」
「た、確かに！」
「あとは、メッセージ作成時に、スレッドIDを指定しないで作成している場所の調査だけど……」
「ちょっと grep しても全部は洗いきれず、つらいです」
「そういう時は、 **汚いかもしれないけど、ログを仕込んでみよう。スレッドIDが未指定のまま before_validation まで進んだら、 `caller` を記録する処理を書くんだ**」

運用中のコードで、コードを読んでもわからない時は、**とりあえずログを仕込む** 方法が効果を上げることがあります。
今回のケースもそうでした。
春にやった修正のように、 **全てのメッセージ作成箇所でスレッドIDを明示的に指定する修正** をすることになったのです。

「あとは、そうだな。１週間くらいログを眺めて、**ログが新しく増えなければ、改修は完了したと言っていいだろう**」

```diff
message = proposal.messages.build(
  body: 'hoge',
-  receiver: User.find(params[:receiver_user_id])
+  message_thread_id: proposal.message_thread.id
)
message.user = current_user
message.save
+ # user_messages は、 thread#users を元に作成する
```

この修正をやりきることで、メッセージのコールバックは大幅に消えることになったのです。

```ruby:app/models/message.rb
class Message < ActiveRecord::Base
  has_many :user_messages

  belongs_to :thread, class_name: 'MessageVirtualThread'

  before_validation :build_user_messages

  after_create :touch_proposal

  delegate :proposal, :job_offer, to: :thread

  def build_user_messages
    thread.users.each do |subscriber|
      user_messages.build(
        user: subscriber,
        thread_id: thread.id,
        read_flag: (self.user == subscriber),
      )
    end
  end

  def touch_proposal
    if proposal.some_cond?
      proposal.update_column(:status, Proposal::SOME_STATUS)
    end

    if !proposal.other_cond?
      if !proposal.replied? && user_id == proposal.receiver.id
        proposal.reply!
      else
        proposal.touch
      end
    end
  end
end
```

## このコードの片隅で

「先輩！終わりが見えましたね！」
「ああ、あとはテーブル名の変更とか、もう使わなくなったカラムの削除とかだね」


「しかし疲れた……」
「お疲れ様」
「あ、ボス」


「なんというか、元のコードとしてクソコードを書いてしまってすみませんでした！」
「いや、それは勘違いだよ。元のコードは **クソコードではない**。その時その時では、最善を目指したコードだよ」


「そもそも、ユーザ様へのUX改善結果と、最初の設計とがずれていったのは仕方ないことじゃないかな？ **システムなんて、運用してみればわからないことはたくさんあるよ。大事なのは、ちゃんと使いやすく変化してくことじゃないかな？**」


「負荷対策のコードだって、別に場当たり的ってわけじゃないさ。**技術的投資だよ。事実、３年半運用できたわけだろ？**」


「**ちゃんと運用できたからこそ、リファクタリングに工数を取れるだけの余裕ができたんだよ。だから、あのコードは技術的投資だ。**」


「とはいえ、どこかで投資は回収しなきゃだめで、今回はそのタイミングだったわけだよ。でもね。**回収する前に、コードの価値がなくなってしまうことだって多いんだ** それこそ、サービスを閉じることになったり、ピボットすることになったり。そう考えると、ちゃんとリファクタリングする段階まで行った元のコードは、**きちんと価値を生んだ、良いコードだった**と言えるんじゃないかな」


「でもまぁ、次は良いコードを書こうと思うなら、それはいいことだよ。なんだかんだ言ったって、先は長いんだ。ゆっくりと良いコードをこれからも目指していこう」

----

くぅ～疲れましたw これにて完結です！
実は、ネタ出ししたらGoサインが出たのが始まりでした
本当は話のネタなかったのですが←
ご厚意を無駄にするわけには行かないのでポエムで挑んでみた所存ですw
以下、クラス達のみんなへのメッセジをどぞ

`MessageVirtualThread` 「俺が登場する前に構造変えとけばよかったな！」

`Message` 「使わなくなったコードは消せよ！」

`Proposal` 「いいか、疎結合だ。疎結合にするんだ」

----

**俺はなんでこんな記事を書いてしまったんだ。次はまともな技術記事書こう……**