【実録】気づいたらやってしまってるかもしれない、こんなコーディング

はじめに

表明駆動投稿ということで、何も考えず投稿表明をして、当日を迎えてしまいました。@suger4です。
この記事はラクス Advent Calendar 2022の13日目です。

あるきっかけで、「コードのプロジェクトを分ける。≒一部を外だしする。」ことになったのですが、そこそこ長く開発していたプロジェクトであったこともあり、何点か事件が発生してしまいました。
今回はそういった事例を紹介させていただき、反面教師になっていこうという回になります。

ことの発端

「コードのプロジェクトを分ける。≒一部を外だしする。」きっかけは、ざっというと「一部の重たい機能を別サーバで動くようにしよう」といった内容です。
特に珍しくないありふれた話ですが、スパゲティのように絡まっている（かもしれない）コードから、確実に、今までと同じように動作するように外に出すことが最大のミッションです。
しかしながら、そう簡単にはいきません。

Case1：相互依存で相手がいないとダメになるメンヘラなクラス

ここにCSVに関するいろいろな処理ができるCsvCommonクラスがいます。
今回はこのCsvCommonクラスを外に出していきたいと思います。

CsvCommon.java

class CsvCommon {
  CSVを読み取る();
  CSVを出力する();
  CSVの拡張子かチェックする();
  CSVを配置するパスを作る();
}

一方こちらにはファイルに関するいろいろな処理ができるFileCommonクラスがいます。

FileCommon.java

class FileCommon {
  ファイルを読み取る();
  ファイルを出力する();
  ファイルの拡張子をチェックする();
  ファイルを配置するパスを作る();
}

（いろいろ言いたいことがあるのはわかりますが）一見、それぞれで処理しているように見えるので、外に出してしまおうと思います・・・
が、中身がこうなっていたらどうでしょうか。

CsvCommon.java

class CsvCommon {
  CSVを読み取る();
  CSVを出力する();
  CSVの拡張子かチェックする();
  CSVを配置するパスを作る(){
    // ある階層までは共通
    path = FileCommon.ファイルを配置するパスを作る();
    // CSV用のファイルパスを作る
  };
}

FileCommon.java

class FileCommon {
  ファイルを読み取る();
  ファイルを出力する();
  ファイルの拡張子をチェックする(){
    // CSVファイルか？
    CsvCommon.CSVの拡張子かチェックする();
    // PDFファイルか？
    /* etc...*/
  };
  ファイルを配置するパスを作る();
}

CsvCommonとFileCommonが相互依存してしまっていて、CsvCommon.javaだけを外のソースに出すわけにはいかないようです。

上の例なら、FileCommon.javaに拡張子チェックをさせるで解決しそうですが、現実のコードでは他にもガッチリと相互依存していて別れさせるのは非常に困難を要します。
別れさせるためのアプローチとしては、どこの処理が相互依存しているのかの洗い出し、影響範囲の確認のためにFileCommon.javaの利用箇所を調査。などいろいろ大変なことになります。
処理を分けることが難しい場合には、CsvCommon.javaと一緒にFileCommon.javaも外に出すことになるやもしれません。

長く育まれた相互依存を解消させるのは難しい。。。そんなケースとなりました。

Case2：みんなに頼られすぎているUtilクラス

こちらにシステムで共通利用する定数を持ったMasterUtilクラスがいます。

MasterUtil.java

class MasterUtil {
  // バージョン
  public static final String SYSTEM_VERSION = "v3.0.0"

  /**
   * ユーザ種別
   */
  enum UserType {
     ADMIN();
     EMPLOYEE();
  }
  /* etc...*/
}

ユーザ種別はシステム共通で使うのでこのクラス内でEnumとして持っているようです。
他にもいろいろな共通Enumが定義されていて、このクラスさえ見ればシステム内の「種別」は網羅的に見れるようになっている気がしますが、長く作り続けられてきたシステムなので、軽く1,000行は超えており、あまり積極的に見たいとは思えません…

さて、今回は上記から外に出すために必要なEnumだけを取り出したいです。今回の活動を機に各Enumも1つのクラスにしてしまおうと思います。

が、MasterUtilクラスはみんなが使っていい便利クラスです。
当然のごとく、多方面からいろいろな用途で使われていて、外部に持っていかれてしまうと残されたクラスたちは困ります。
最終的には必要なEnumをコピーして外部にも持つことになり、また一つ管理するものが増えてしまいました…

ただ今回のケースは一長一短で、ある種外に出した機能にとって不要なものはなくなり、元のコードに何か新しく追加したとしても、それが外に出した機能で不要ならば律儀に同じようにメンテナンスする必要はないと思います。
アプローチの方法は違うような気がしますが、ドメイン駆動設計で言われる境界付けられたコンテキストに近いものを感じました。

まとめ

いかがだったでしょうか、あるあるだね～。よく聞く話だね～。といった感想かと思います。

私自身もいろいろな書籍で「こんなコードはアンチパターンだ」「疎結合に高凝集なコードにせよ」といった言葉を目にし、その痛みを書籍からは感じてはいましたが、直接対面してみるとなかなかに頭を悩ませてくるものでした。

知識として、こういったアンチパターンの理解はあるものの、現実問題として実装しているときや、レビューしているときは「このロジックで正しく業務はなされるのか？」といった業務の本質に注意が向いてしまうものです。（私はそうあるべきだと思います。）

ドメイン駆動設計などでは、実装を行う前に「ドメイン」としてどこまでがその業務の範囲なのかをモデル化することで、必要十分な依存関係に押しとどめ、各ドメインの結合を疎にすることができる。という部分に価値があるのかなと考えています。

雑多でしたが、本投稿が皆さんのコーディング活動に一役買えれば幸いです。

以降のラクス Advent Calendar 2022の投稿もお楽しみに！！