【備忘録】コードを書くときに気をつけること（１）

最近，「良いコード／悪いコードで学ぶ設計入門―保守しやすい　成長し続けるコードの書き方」という本を読んでいるので，コードを書くときに気をつけるべきことを備忘録として書いておく．とりあえずは新たに学んだ気をつける点を列挙してく形になるが，本を読み終わってからは「〇〇で困っている→なら△△を使う！」というように辞書引きできる形にしようと思う．
https://www.amazon.co.jp/dp/B09Y1MWK9N

クラスは単体で完結させる（低・高凝集）

よくない例

コードを書いていてよくあるのが，あるクラスの初期化やその他の処理などに別のクラスの変数などが依存してくるという状況だ．例えば，「データクラス」というデータを保持（というか集約）を目的としたクラスなどを作り，そのデータを使った処理をまた別のクラスで定義する場合などがそれにあたる．

class Product:
    def __init__(
        self,
        product_id: int,
        name: str,
        price: int,
        category: str,
    ):
        self.product_id = product_id
        self.name = name
        self.price = price
        self.category = category

class PriceProcess:
    @staticmethod
    def is_valid(price):
        return price > 0

    @staticmethod
    def get_tax(price):
        return price * 0.1

ちょっといい例が思いつかなかったが、上の例では商品クラスの初期化のために別の価格処理クラスが必要となっている。だが、商品クラスがデータクラスのようにデータの集約のみをしていて、他に処理クラスに処理をまとめているのでちょっと良さそうに感じてしまう。（上の状況だとおかしいのはわかるが、もうちょっとそれっぽい状況だとよくやってしまう。）
言いたいこととしては、「データはデータ、処理は処理」というまとめ方をすると、クラス単体で動作しなくなっちゃうよっていうこと。もっと簡単な例だと、あるクラスの変数のバリデーションをどこか別の場所に定義したメソッドでやってしまう、みたいな感じ。

良くないところ

良くない点としては最初に上げたものも含め，

クラス単体で完結しない

他の処理に依存していると「ここを変えるとどんな影響が出るかわからない...」となり影響の調査に余計な時間がかかる．

関係している機能なのに書かれる場所が離れる

基本的に，関係する機能であれば同じタイミングでコードを編集することが多いと思うが，それぞれが離れた位置に書かれていると編集がしづらい（これは正直複数タブ開けばいいだけだが，本来はわざわざそういったことをしなくてもいいのが理想なはず）

他の人がクラスを使用しづらい

「単体で完結しない」にも関係するが，そのクラスが動作する条件を読み解かなければ使えないので時間がかかる．これは割と大きな問題で，「自分は覚えているから影響ない」という精神だと自分の時間は短縮できても他の人の時間を大幅に奪ってしまうことになる．

などが考えられる．

改善方法

改善するときは，商品価格に関するものはデータと処理など全て近い場所に（というか同一クラスに）置くのが効果的となる．こういった関係の強いものを集めたような設計を高凝集というらしい．

商品価格クラス

class ProductPrice:
    def __init__(
        self,
        price: int
    ):
        self.price = price
	if not self.is_valid(self):
	    raise HogeError("hoge")

    def is_valid(self):
        return self.price > 0

    def get_tax(self):
        return self.price * 0.1

商品クラス

class Product:
    def __init__(
        self,
        price: ProductPrice
	...
    ):
        self.price = price
	...

一つの多機能クラスをつくらない（密・疎結合）

よくない例

よくあるのが「このデータ読込用の関数は色んなとこで使うから，utilsに定義してそこから参照しよう！」という感じで，よく使う関数をとりあえずutilsにおいてしまう状況だ．実際に，その瞬間は関数の置き場所を悩まないし開発が早く進む気がするが，あとから「やっぱりこの関数の機能ちょっと変えたい...」となった時にリポジトリ全体に散らばったその関数を使っている全ての場所に影響がないか考えなくてはならない．

train.py

train_data = load_data(train_path)
valid_data = load_data(valid_path)

trainer.train(
    train_data,
    valid_data,
    ...
)
...

evaluate.py

test_data = load_data(Path(test_path))
result = evalate(model, test_data)
...

utils.py

def load_data(path: Path):
    df = pd.read_csv(path)
    return df

上は，機械学習プロジェクトでよくあるコードで，データの読み込み用の関数をutilsにおいて学習・評価用コードから呼び出している．もちろん，この関数一つであればなんの問題もないが，プロジェクトが大きくなると共にutils内の関数もどんどん増えていき，いろいろなコードがutilsを参照するようになる．そうなると，影響の範囲が非常に広くなり少しの変更でもいろいろなバグが発生する可能性が出てくる．

良くないところ

変更に弱い

構造的に一つのファイルに参照が集まりすぎて，少しの変更でもリポジトリ全体に影響が出てしまうことになるので，変更をするのに非常に時間がかかる．

修正漏れが発生する

utilsを参照するファイルからは定義元がわかりやすいが，一方でutilsからはどこでその関数が使われているか把握しづらい．そのため，定義元のutilsを修正するとどこかのファイルでは想定外の使い方をしていて急に動かなくなるかもしれない．

改善方法

これは単純に「なるべくutilを作らない」というのが一番有効な改善方法となる．正直utilsは便利なので作りたくなってしまう気持ちはあるが，それをぐっとこらえて「その機能（メソッド）はどのクラスが持つべきか？」を一度考えるのが良いと思う．一つの多機能なクラスを作っていろいろな場所から参照される形を「密結合」といい，逆に，それぞれの機能を適切な場所に集めてまばらに参照（依存）させることを「疎結合」という．下の例は「疎結合」となる．

dataset.py

class HogeDataset:
    def __init__(self, train_df, valid_df):
        ...
	
    @classmethod
    def load(cls, train_path, valid_path):
        train_df = pd.read_csv(train_path)
	valid_df = pd.read_csv(valid_path)
	
	return cls(train_df, valid_df, test_df)

train.py

hoge_dataset = HogeDataset.load(train_path, valid_path)
...

上の例では，そもそもデータ読込関数はそのデータのクラスに依存しているものなのでそこに配置している．ただ，データセットの読み込み処理は似たようなコードになることが多く，一見すると重複したコードができてしまいSOLID原則に反するように見えるが，それは「たまたま同じ読み込み処理になっている」だけで全く違う処理になる可能性が大いにあるため共通化するべきではない．

とりあえず半分くらい読んだ中で新たに覚えた気をつけるべきことを書いてみた．