どこまで最適化するのが最適なのかって話

はじめに

初めまして！
皆様どうもこんにちわ、こんばんわ、おはようございます。
エンジニアの enomoto です。

最近、新規開発、1=>10 程度の開発、その他様々なコードもみてきて、リファクタリングや設計について考える機会があり、どこまで最適化（分離）するのが最適なのかという問題について改めて考えさせられました。

特にクラスやメソッドの分離において、完璧な設計を目指すことと、シンプルさを保つことのバランスについて備忘録として記録しておきます。

※この記事でいう最適化というのは主に分離や見やすさなどの話で、パフォーマンスの最適化などは含んでいません。
※個人的な意見なので、そんな考えもあるんだーぐらいで見ていただければ幸いです。

最適化について

クラス・メソッド分離における最適化とは

ここでいう最適化とは、コードをより読みやすく、保守しやすくするために行うクラスやメソッドの分離・抽象化のことを指します。

よく言われる設計原則として以下があります：

• 単一責任原則: クラスは一つの責任のみを持つべき
• DRY 原則: Don't Repeat Yourself - 同じコードを繰り返さない
• 関心の分離: 異なる関心事を別々の場所で扱う
• YAGNI (You Aren't Gonna Need It): 今必要でない機能は実装しない。将来の拡張性を考えすぎて、今は不要な複雑さを導入しない
• KISS (Keep It Simple, Stupid): シンプルに保つ。複雑な設計より、理解しやすい設計を優先する

これらの原則は確かに重要ですが、最初から完璧に適用しようとすることには落とし穴があります。

この辺りをガッチガチにやるのは一定エンジニアのエゴ的なところもあり、自分が思う最適に近づけようと思うこともよくありますよね。
こういうのを考えるのがコードを書く楽しさみたいなところもあり、プログラマーの楽しさでもありますが。。

自分はそういうのを考えるのが好きでエンジニアになったみたいな部分もあります。
これが現状の最適な一手だ！神の一手を極めるぜ！みたいな。(ヒカルの碁参照)

これはここまで分離されているべきだ!
ここまでやっておいた方が機能が追加されたときに楽になる!
など、自分の経験やこれまで書いてきたコード、携わったプロダクトの状況なども含めてそれらの考えは育まれてきているはずです。

生成AIの影響

そして生成AIの発展により、複雑な設計パターンを簡単に実装できるようになりました。

しかし、AIがそれっぽいコードを書いてくれる一方で、個人的には以下のような問題も生じてきているという感覚があります：

• 多すぎる try catch - 必要以上の例外処理
• 謎のテストコード - 一応通るが意味のないテスト
• 過剰な抽象化 - まだ必要のない将来対応

このため、今のアプリケーションではどこまでやるべきなのかをエンジニアが適切に判断する必要があります。

分離する前と後のコード例（Ruby）

具体例として、注文処理機能を 2 つのパターンで実装してみます。
(他にもかなり具体例のパターンはあると思います)

パターン 1：全てを 1 つのメソッドに詰め込んだコード

class OrderProcessor
  def process(order_data)
    # バリデーション
    if order_data[:items].nil? || order_data[:items].empty?
      puts "注文アイテムが必要です"
      return false
    end
    if order_data[:customer_id].nil?
      puts "顧客IDが必要です"
      return false
    end
    if order_data[:shipping_address].nil? || order_data[:shipping_address].strip.empty?
      puts "配送先住所が必要です"
      return false
    end

    # 合計金額計算
    total = 0
    order_data[:items].each do |item|
      if item[:price].nil? || item[:quantity].nil?
        puts "商品の価格または数量が不正です"
        return false
      end
      total += item[:price] * item[:quantity]
    end

    # 税金計算
    tax = total * 0.1
    final_total = total + tax

    # 在庫チェック
    order_data[:items].each do |item|
      stock = get_stock(item[:product_id])
      if stock < item[:quantity]
        puts "在庫不足です: #{item[:product_name]}"
        return false
      end
    end

    # 注文保存
    begin
      order = Order.create!(
        customer_id: order_data[:customer_id],
        total_amount: final_total,
        tax_amount: tax,
        shipping_address: order_data[:shipping_address],
        status: 'pending'
      )

      order_data[:items].each do |item|
        OrderItem.create!(
          order: order,
          product_id: item[:product_id],
          quantity: item[:quantity],
          price: item[:price]
        )
      end
    rescue => e
      puts "注文の保存に失敗しました: #{e.message}"
      return false
    end

    # 在庫更新
    order_data[:items].each do |item|
      update_stock(item[:product_id], item[:quantity])
    end

    # メール通知
    begin
      OrderMailer.confirmation(order.id).deliver_now
      OrderMailer.admin_notification(order.id).deliver_now
    rescue => e
      puts "メール送信に失敗しました: #{e.message}"
    end

    # ログ出力
    Rails.logger.info "注文が完了しました: Order ID #{order.id}, Customer ID #{order_data[:customer_id]}, Total: #{final_total}"

    puts "注文が正常に処理されました"
    true
  end

  private

  def get_stock(product_id)
    # 在庫取得ロジック
    Product.find(product_id).stock_quantity
  end

  def update_stock(product_id, quantity)
    # 在庫更新ロジック
    product = Product.find(product_id)
    product.update!(stock_quantity: product.stock_quantity - quantity)
  end
end

問題点：

• 1 つのメソッドが 50 行を超えて非常に長い
• バリデーション、計算、保存、通知が全て混在している
• エラーが発生した場所の特定が困難
• テストが書きにくい
• 修正時に他の処理に影響を与えるリスクが高い

パターン 2：適切にメソッドを分離したコード

class OrderProcessor
  def process(order_data)
    return false unless valid_order?(order_data)

    total_amount = calculate_total_amount(order_data[:items])
    return false unless sufficient_stock?(order_data[:items])

    order = save_order(order_data, total_amount)
    return false unless order

    update_inventory(order_data[:items])
    send_notifications(order)
    log_order_completion(order)

    puts "注文が正常に処理されました"
    true
  end

  private

  def valid_order?(order_data)
    if order_data[:items].nil? || order_data[:items].empty?
      puts "注文アイテムが必要です"
      return false
    end

    if order_data[:customer_id].nil?
      puts "顧客IDが必要です"
      return false
    end

    if order_data[:shipping_address].nil? || order_data[:shipping_address].strip.empty?
      puts "配送先住所が必要です"
      return false
    end

    true
  end

  def calculate_total_amount(items)
    total = 0

    items.each do |item|
      if item[:price].nil? || item[:quantity].nil?
        puts "商品の価格または数量が不正です"
        return nil
      end
      total += item[:price] * item[:quantity]
    end

    tax = total * 0.1
    total + tax
  end

  def sufficient_stock?(items)
    items.each do |item|
      stock = Product.find(item[:product_id]).stock_quantity
      if stock < item[:quantity]
        puts "在庫不足です: #{item[:product_name]}"
        return false
      end
    end
    true
  end

  def save_order(order_data, total_amount)
    begin
      order = Order.create!(
        customer_id: order_data[:customer_id],
        total_amount: total_amount,
        tax_amount: total_amount * 0.1 / 1.1,
        shipping_address: order_data[:shipping_address],
        status: 'pending'
      )

      order_data[:items].each do |item|
        OrderItem.create!(
          order: order,
          product_id: item[:product_id],
          quantity: item[:quantity],
          price: item[:price]
        )
      end

      order
    rescue => e
      puts "注文の保存に失敗しました: #{e.message}"
      nil
    end
  end

  def update_inventory(items)
    items.each do |item|
      product = Product.find(item[:product_id])
      product.update!(stock_quantity: product.stock_quantity - item[:quantity])
    end
  end

  def send_notifications(order)
    begin
      OrderMailer.confirmation(order.id).deliver_now
      OrderMailer.admin_notification(order.id).deliver_now
    rescue => e
      puts "メール送信に失敗しました: #{e.message}"
    end
  end

  def log_order_completion(order)
    Rails.logger.info "注文が完了しました: Order ID #{order.id}, Customer ID #{order.customer_id}, Total: #{order.total_amount}"
  end
end

良い点：

• 各メソッドが単一の責任を持っている
• 処理の流れがprocessメソッドで明確に把握できる
• 個別の処理をテストしやすい
• 修正時の影響範囲が限定的
• エラーの原因を特定しやすい

どこまで最適化するのが最適なのか

上記は極端な例で、もちろんパターン 2 の方がわかりやすい、見やすいというのはエンジニアとして大前提です。

ですが、これが今のアプリケーションにとって最適なコードなのか？と問われるともう少し別の判断軸も必要だと考えます。

判断基準となる要素

それは以下の要素に依存します：

• プロダクトのフェーズ: ローンチ前なのか？ローンチ後なのか？
• ユーザー規模: ユーザー数は何人なのか？データ量はどれだけある？
• チーム体制: 開発者は何人なのか？
• 会社の状況: 資金やリソースの制約はあるか？

状況別の判断例

🚀 急成長期・資金制約がある場合

• 優先事項: 一刻も早くローンチすること
• コード方針: とりあえず動けば良いコード（パターン 1 でも問題なし）
• 理由: ロジックが正しければ、世間一般でいうアンチパターンでも価値を提供できる

📈 成長軌道・チーム拡大期の場合

• 優先事項: 持続可能な開発体制の構築
• コード方針: 見やすく分離されたコード（パターン 2 への移行）
• 理由: 複数の開発者が効率的に作業できる環境が必要

実際の問題が起きてからリファクタリングする

最初はシンプルに始める理由：

1. 要件が不明確: 初期段階では要件変更が頻繁に起きる
2. 過度な抽象化のリスク: 使われない抽象化は技術的負債となる
3. 開発速度: シンプルなコードの方が素早く機能を実装できる

分離のタイミング

以下のような状況になったら分離を検討します：

1. メソッドが長くなりすぎた場合

# 20行を超えるメソッドは分離を検討
def complex_method
  # 20行以上のコード...
end

2. 同じロジックが複数箇所で使われている場合

# 重複するバリデーションロジック
class UsersController
  def create
    return render :new unless valid_email?(params[:email])
    # ...
  end
end

class AdminUsersController
  def create
    return render :new unless valid_email?(params[:email]) # 重複！
    # ...
  end
end

3. テストが書きにくい場合

テストが書きにくいというのは分かりやすく気づけるタイミングです。
mock しやすい形になっていない。
依存関係が多すぎるなどもポイントですね。
自分の違和感(これのテスト書くの面倒だな)を大事にして、嗅覚を育てましょう。

# 外部APIへの依存があり、テストが困難
def create_user_and_sync_to_external_api
  user = User.create!(params)
  ExternalAPI.sync_user(user) # テストが困難
  user
end

終わりに

コードは生き物であり、会社や需要とともに成長していくものです。
最初から完璧である必要はなく、むしろ変化に対応できる柔軟性の方が重要です。
現時点で何を代償にして、何を得るのかのトレードオフをよく考え、最適なコードを書く。

そういったバランス感覚を養うことが、持続可能な開発につながると考えています。