kagamihogeの日記

kagamihogeの日記です。

継承はしご

Struts アプリの保守はそろそろキツくなってきたのかもなぁ・・・。実装継承を前提にした設計で組むってのは、最初はいいんだけど後になって機能追加やらなんやらでコードにアレコレ手を入れる段になると色々キツい面があるのでは、って気が最近してきた。

まずは、今回のエントリの基点となるコードから。

public class A {
public void hoge() {
hogeSub1();
hogeSub2();
}

protected void hogeSub1() {
}

protected void hogeSub2() {
}
}

Template Method なコード。Struts のコードに合わせてないのでちと難があるんだけど、古めの web アプリではよくやる手。継承階層のいちばん上のクラスがログイン中かどうかの検査とかの共通部分を持ってて、サブクラスが各々必要な処理を実装する、ってタイプ。

まぁこんな感じのアレ。

public Class BaseAction extends Action {
public ActionForward execute(...) {
doCommon(...);
doExecute(...);
}
protected void doExecute(...) {
//subclass responsibility
}

public Class HogeAction extends BaseAction {
public void doExecute(...) {
...
}
}
}

それはさておき、今回の失敗談の続き。↑の Struts の例みたく A も継承されて差分プログラミングチックなことが行われる。

public class B extends A {
protected void hogeSub1() {
...
super.hogeSub1();
...
}
}

この時点で B1 の挙動を B1 のソースコードだけで予測するのは難しくなりやすい。といっても、一階層だけなら doExecute のようなあらかじめオーバライドされることが決められているメソッドだけ継承する場合なのでそれほど混乱することはない。

ここからが厄介な部分。B をさらに拡張した C が登場する。

public class C extends B {
public void hoge() {
...
super.hoge();
...
}

protected void hogeSub1() {
...
super.hogeSub1();
...
}

protected void hogeSub2() {
...
hogeSub1();
...
}
}

説明のためにかなり単純化したコードになってはいるけど、実際はもっと色んなコードにまみれながら super の呼び出しがちょこんと置いてあるところを想像していただきたい。そんな状況で、C のソースだけでこのクラスの挙動を推測出来たら個人的にネ申。

実際 C を読みすすめてるときは・・・「えーと hoge は super の hoge 呼んでるのか。で、B の hoge は・・・って無いじゃん。あぁ、さらに上位のクラスのメソッドなのねコレ。んで、そのいっちゃん上位の hoge は hogeSub1, hogeSub2 を呼び出している・・・でもって C で hogeSub1, hogeSub2をオーバーライドしてるな。次は hogeSub1 か・・・って、hogeSub1 も super 呼び出ししてるのか。コイツは B の hogeSub1 に行って A の hogeSub1 まで昇る、と。hogeSub2 は hogeSub1 呼び出すのか。んで結局 C の hoge 呼ぶとどんな動きになるかっていうと・・・ あれ?ちょっとまてよ C で hoge オーバライドしていて、そん中で super.hoge して hoge は hogeSub1 と hogeSub2 呼び出してそいつらはオーバライドされててqあwせdrftgyふじこlp」

ヘタに実装継承すると goto スパゲッティなプログラムと似たような感じになるのかもしれない。
まぁ、今回のケースはそれ以前の次元の問題かもしれんけど。


なんでこんな実装継承のハシゴになってるのか、について。「動いてるソースには手を入れるな」を地でいった結果、というのが俺の考え。

既存のソース崩さずにアクションに追加機能付与しようと思ったら実装継承しか手がない。アクションにビジネスロジックがベタ書きしてるなら尚更。もしクラスに切り出してあったとしてもアクション内のコードを再利用しようと思ったら継承しか手がないし。既存クラスのコードをリファクタリングして別クラスに移そうにも、まともなテストコードがない状態だと結構なリスクがあるし・・・。

更に悪いことに、アクションの一部だけ止む無く拡張する必要が生じた際に、その部分だけ protected メソッドに切り出し→継承→先に protected にしたメソッドをオーバーライドなんてことをやっている。これによって延々 Template Method モドキの階層が深まることになってしまった。super の迷宮が非常に読みにくいのはさっき見た通り。これはかなりの失敗だったと思う。

まぁ「実装継承って実はヤバイんじゃね?」ってのが世間にやっと広まり始めるより前から存在してるコードだからしょうがないんだけどさ。

つーわけで Mix-in とかパーシャルクラスについて詳しいとこをもちっと抑えとかないとな、と思いました。