現場で出くわした残念なコードシリーズ【副作用①】

この記事で伝えたいこと

既存コードの残念な状態（setterによる副作用）を許容しながらもインスタンス内部で整合されているべき状態が不整合にならないようにリファクタリングするパターン

どんなコード？

@AllArgsConstructor
@Getter
@Setter
public class EstimateDetailItem {
    private int unitAmount;
    private int quantity;
    private int totalAmount;
}

public class ZannenCode {
    public void main() {
        final EstimateDetailItem itemA = new EstimateDetailItem(10, 10, 10 * 10);
	// ・・・なんかいろいろ処理
    }
}

今回の残念はどこ？

見方によっていろいろあるとは思うのですが、今回は以下２点に着目したいと思います。

• setterによる状態の副作用
• setterによる副作用によって保持している内部の別の状態と同期されるべき値との同期の担保が取れない状態

どうして残念なのか？

setterによる副作用があるせいで、インスタンス内部の整合性の担保が常に取れている保証が取れない（※取るためにはすべてのsetterの呼び出し箇所を人力捜査しなければならない）という点です。

// EstimateDetailItemは内部状態として常に以下が成り立っていないとおかしい
totalAmount = unitAmount x quantity

上記の整合性はいとも簡単に破ることができてしまいます。

itemA.setUnitAmount(11); // =>内部的には 11 x 10 = 100 不整合

問題はどこにある？

前述しているようにまず、根本的な問題はsetterによる副作用です。
インスタンス生成時点で整合性が一度保たれた時点でそれ以降の状態変更を一切許可しない作りにできれば整合性は自然と担保できます。

とはいえ既にあるコードが・・・・・至るとこでsetter呼んでるんです・・・・

わかります！そんなときは
その値がインスタンス内部の他の値から計算によって算出できないか？を考えてみてください。

今回の例でいえば
EstimateDetailItemのtotalAmountは内部状態に存在するunitAmountとquantityの計算結果として導き出せます。そんなときは、内部で状態を持たずに呼び出し元が値を要求した時に常に最新の依存値で再計算するgetterだけを内部に残すことで副作用を残したまま、整合性を担保できる形に持っていけます。

@AllArgsConstructor
@Getter
@Setter
public class EstimateDetailItem {
    private int unitAmount;
    private int quantity;
-   private int totalAmount;
    
+   private int getTotalAmount() {
+     return unitAmount * quantity
+   }
}

こうすることで副作用を致し方なく残しつつも部分的に整合性の担保された状態を増やすことができます。

final EstimateDetailItem itemA = new EstimateDetailItem(10, 10);
itemA.setUnitAmount(11); // どこで何をセットされても
// ・・・

// 欲しいと思ったところ
itemA.getTotalAmount() // 常に欲しいと思った瞬間の最新値で再計算された整合性のとれた値を返してくれる。

まとめ

かなり基本的なリファクタリングテクニックですが、エンジニアになって5年くらいですが、かなり頻繁に私は使うテクニックです。（※そんなコードばっかと向き合ってるのは辛いですが・・・）