開発するときに心がけていること 実装編

この記事は株式会社SUPER STUDIOのAdvent Calendar 2023の21日目の記事です。

初めに

先日チームメンバーと設計の話をした時に「どういう時にクラスを分けるのか？」という話をチームでした。自分なりの判断基準はあるのだが、それを明文化したことがなかったのでしてみようと思う。 今時点での思考なので、この先変わる事もあり得るので「こういう考えもあるな」くらいの気持ちでとらえていただけると嬉しい。

分割

ケース1: このクラス、やる事多くない？

SOLID原則の「単一責務の原則」を基準に考えて、一つのクラスでやることが多いと感じたらクラスを分ける。 例えば商品の値段を計算する処理と商品の割引の値段を計算する処理、送料を計算する処理が一つのクラスの中でやっていたとすると、それらは別々のクラスにする。 割引の処理に新しいロジックが追加された時、予めその処理を別のクラスに抽出していたら変更する範囲はそのクラスの中だけになるし、そのクラスのテストをちゃんと書いておけば良くなる。 以前ある人から聞いたのだが、「このクラスはXXをYYするクラスです」と一言で説明できる単位が良いのだろうと思う。

ケース2: このメソッド、色々やりすぎじゃない？

例えばメソッドの名前が「get~~」ってなっていて確かにその値をreturnしているけど、中身で他の処理（データの更新など)をしていたりするとメソッドを分割することを検討すると思う。 副作用なども考えると思う。

共通化

ケース1: これって処理同じだから共通化しようか？

ただ同じだからという理由で処理を抽出するということはしないと思う。 ドメインロジックは共通処理として切り出すが、それ以外については結構慎重になる。 共通化しても後々の開発でその共通処理に分岐が 入って逆に処理が複雑化することもある。 重複するところがあるが作りがシンプルになるのであれば、敢えて重複は許容します。

ケース2: そうだ継承しよう

継承を使って処理を共通化しようというケースもある。それがハマる時は良いが、後々処理が追加された時に継承していることによって逆に複雑度が上がる場合がある。 例えばRailsにSTIと言う仕組みがある。Imageモデルがあり、それを継承してCustomerImageとAdminImageがあるとする。 CustomerImageとAdminImageもどちらもImageテーブルにデータが入りtypeと言うカラムで二つのデータの区別をする。 当初は良かったが、途中からAdminImageだけに別のデータを持つことになりカラムhogeを追加したとする。 するとCustomerImageの場合はhogeはnilにAdminImageは何かしら値が入る。そういう拡張が少しずつ増える・・・・。 そうなると、そもそも共通化する意味があったのか。継承したことでCustomerImageとAdminImageを分けたくても出来ない。 そういうことが起こる場合がある。 継承するくらいなら処理を別クラスに移譲し各々のクラスでそれを使うという方が良いかもしれない。

ケース3: テストも共通化しよう

テストの可読性を上げようということで共通化するといこともある。Rails、Rspecで開発している場合にそのための仕組みもある。 で、私も色々と試行錯誤したが、結果テストでDRYを追求する必要はないと思っている。テストでは「この処理はどういうデータが必要で結果どうなるのか」それが1ファイル(1context)で完結するのが良いと思っている。 shared_contextを使うと確かにテストはスッキリするが、テストの内容を把握するのに共通部分として切り出したところを読み、そしてそれを踏まえてテストファイルの方を見る。 それよりもテストの内容は1ファイルで、1contextで完結していた方が必要なものを把握し内容が理解しやすいと感じた。 これは個人差があると思うが、他にもそういう風に感じている人はいそうだった。 ポイントは、数ヶ月後に自分が見ても内容が把握しやすいテストであれば良いと思っている。

まとめ

色々あるが、根底にあるのは「数ヶ月後に見ても理解しやすいか」ということ。 メンテし続けるコードなら、そのためにどうあるべきかを考えている。これからもそのことを念頭に開発していきたいと思う。 (他にも何か書こうと思ったので、思い出したら追記します)