保守の観点から見るオブジェクト指向レベルの量り方:追記

ブクマありがとうございます。

これほどブクマ頂いたのは初めての経験で、嬉しくなったのでブコメに反応します。

(ちなみに本文は文語表現で追記は口語表現である事に特に意味はありません。ちょっと文語表現使いたくなっただけ)

「オブジェクト指向レベル」って何だろう

はてなブックマーク - Bookmark life like a clown - 2009年10月15日

・・・タイトルの件はスルーして下さい。お願いします、お願いします・・・

いい例の方も教えてほしいなぁ.

はてなブックマーク - HISAMATSUのブックマーク - 2009年10月15日

教える程の技量は無いですが、ブコメ&コメント&TBを参考に、個人的意見を加えて良い例の方も考えてみたいと思います。

(ちなみにJava1.4で例をコーディングしています。現場にはJava1.5が導入されていないので)

「1. 参照」例の、より良い実装

最初の例は,getXXX を指摘した方が良いような気がする.getXXX で取得してきた参照に対して変更を加える(必要がある)と言う設計は,あまり良くないように感じる.

はてなブックマーク - Bookmark life like a clown - 2009年10月15日

最初の例は、Bean#addHogeだとよりよい気もする

はてなブックマーク - lizyのブックマーク - 2009年10月15日

BeanクラスのAPIが根本的に悪い、という意見がやはり多いですね。

本文のBeanの実装は以下のコードであるとします。

public class Bean{
    private List hogeList = new ArrayList();
    public List getHogeList() {
        return hogeList;
    }
    public void setHogeList(List hogeList) {
        this.hogeList = hogeList;
    }
}

この実装だと、hogeListプロパティのカプセル化がいないなされていないので、内部を好き勝手にいじられてしまいます。

hogeListに意図する型以外の要素を格納されてしまったり、独自のList実装クラスを渡されてしまったり、

nullを渡されてしまったり・・・

このようなコードばかり保守しているので、本文では特に問題にしなかったのですが、やはり一般的に問題のあるコードである事が理解できました。

ありがとうございます。

以下はそこら辺を考慮して、Beanのあるべき実装を考えてみたものです。

package main.sample;
/**
* (略)
*/
public class Bean {
    private List hogeList = new ArrayList();

    /**
    * 指定されたHogeを、このBeanが保持するhogeリストに追加します。
    *
    * @param hoge hogeリストに追加されるhoge
    * @return この呼び出しの結果、hogeリストが変更された場合は true
    * @throws NullPointerException 指定されたhogeがnullの場合
    * @throws IllegalArgumentException 指定されたhogeがhogeとして有効で無い場合
    *
    * @see main.sample.Bean#isEnableHoge(java.lang.String)
    */
    public boolean addHoge(String hoge) {
        if (hoge == null) throw new NullPointerException("hoge is null");
        if (!isEnableHoge(hoge))
            throw new IllegalArgumentException("parameter is not 'hoge' : " + hoge);
        return hogeList.add(hoge);
    }

    /**
    * このBeanが保持するhogeリスト内の指定された位置にある要素を返します。
    *
    * @param idx 返されるHogeのインデックス
    * @return リスト内の指定された位置にあるHoge
    */
    public String getHoge(int idx) {
        return (String)hogeList.get(idx);
    }

    /**
    * このBeanが保持するhogeリストを、指定されたhogeリストに置換します。
    *
    * @param hoges 置換されるhogeリスト
    * @throws NullPointerException 指定されたhogeリストがnullの場合
    * @throws ClassCastException 指定されたhogeリスト内の要素が文字列で無い場合
    * @throws IllegalArgumentException 指定されたhogeリスト内の要素がhogeとして有効でない場合
    *
    * @see main.sample.Bean#isEnableHoge(java.lang.String)
    */
    public void setHoges(List hoges){
        if (hoges == null) throw new NullPointerException("hoges is null");
        // 防御的コピー
        List newHoges = new ArrayList();
        for (Iterator iter = hoges.iterator(); iter.hasNext();){
        String hoge = (String) iter.next();
        if (!isEnableHoge(hoge))
            throw new IllegalArgumentException("hoges is not list of 'hoge' : " + hoge);
            newHoges.add(hoge);
        }
        this.hogeList = newHoges;
    }

    /**
    * このBeanが保持するhogeリストのコピーを返します。
    *
    * @return hogeリストのコピー
    */
    public List getHoges() {
        // 防御的コピー
        List copy = new ArrayList();
        Collections.copy(hogeList, copy);
        return copy;
    }

    /**
    * 指定された文字列が、hogeとして有効であるか判定します。
    * <br/>
    * 「hogeとして有効な値」とは、「nullでない」かつ「"hoge."という接頭辞で始まる」という条件を満たす文字列を指します。
    *
    * @param value 判定対象の文字列
    * @return 指定された文字列が、hogeとして有効であればtrue
    */
    public boolean isEnableHoge(String value) {
        return value != null && value.startsWith("hoge.");
    }
}

BeanのプロパティであるhogeListに対する操作を、どこまでBeanの公開APIとして実装するかは要件によります。

removeやsetなどとかですね。

個人的に防御的コピーなど単純にオブジェクト参照を返す・渡すセッタ・ゲッタである場合、

メソッド名は「set + プロパティ名」ではなく、上のコードのようにget + 要素論理名の複数形の方が、

通常のセッタ・ゲッタと区別できていいんじゃないかなーと思うのですが、いかがでしょうか?

下記はBeanクラスのクライアントの一例です。

Bean bean = dao.getBean();
bean.addHoge("hoge.AAAA");
bean.addHoge("hoge.BBBB");
//bean.addHoge("CCCC"); メソッド規約違反(hogeとして有効ではない)。実行時例外が発生する。
List list = bean.getHoges();
list.add("sssssss");    // beanには反映されない。
//bean.setHoges(null); メソッド規約違反(指定したhogeListがnull)。実行時例外が発生する。
//bean.setHoges(list); メソッド規約違反(hogeとして有効ではない)。実行時例外が発生する。

「2. メソッドのコメント」例の、より良い実装

2に関して、「データソースから取得する」という意味ならばDao視点ではないか、というご指摘を頂きました。

RE: 保守の観点から見るオブジェクト指向レベルの量り方 - SiroKuro Page

なるほど、確かに。

本文の2に挙げた例では、どちらにせよメソッドの概要としては少し説明不足な気がします。

「Beanのリストを取得する。」よりは、

「指定されたBeanを条件としてBeanのリストをデータソースから取得する。」

「指定されたBeanを条件としてBeanのリストをデータソースから取得して返す。」

などの方がメソッドの概要コメントとして、より適切かなーと思います。

「3. クラスの定義&コメント」例の、より良い実装

例が抽象的すぎて、どのようなケースで使われているかわかりづらくなってしまいました。

3の例は、Controllerに公開されるModel(のビジネスロジック)のインターフェイスとして捉えて頂ければと思います。

(Strutsで例えると、Actionクラスでインスタンス化されるようなクラスです)

実際の処理内容にもよりますが、処理が複雑であれば「interface&骨格実装クラスを定義するパターン」が有効かなと思います。

処理が単にDAOのメソッド呼び出しのみ等であればクラスを分けずにFacade(BisinessDelegate)で一つのクラスに複数のメソッドを定義するのが スマートだと思います。

3.1 interface&骨格実装クラスを定義するパターン

public interface AAAALogic {
    public void execute(Bean bean);
}

public abstract class AAAALogicBase implements AAAALogic {
    public void execute(Bean bean) {
        before(bean);
        executeLogic(bean);
        after(bean);
    }

    public void before(Bean bean){
        // 共通前処理
    }

    public abstract void executeLogic(Bean bean);

    public void after(Bean bean){
        // 共通後処理
    }
}

public class AAAAFindLogic extends AAAALogicBase {
    public void executeLogic(Bean bean) {
    // 「Find」的な処理
    }
}

public class AAAAEntryLogic extends AAAALogicBase {
    public void executeLogic(Bean bean) {
    // 「Entry」的な処理
    }
}

3.2 Facade(BisinessDelegate)で一つのクラスに複数のメソッドを定義するパターン

public class AAAALogicFacade {
    public static void find(Bean bean){
        // 「find」的な処理
    }
    public static void entry(Bean bean){
        // 「entry」的な処理
    }
}

(ここではステートレスなFacadeにしましたが、要件によってステートフルが適す場合もあります。個人的にめちゃくちゃ難しい問題なのでスルーします。)

本文では「クラス」を「処理」と考えるのが悪い、と言っているように読み取れてしまいますね・・・

「クラス」を「処理」と考えるのは悪くなく、むしろ「責務の分離」を果たす為には積極的に利用した方がいいと思います。

(昔言われてたらしい「名詞をクラスに、動詞をメソッドに」という設計手法は、逆にオブジェクト指向設計を妨げる要因になるようです)

「クラス」を「処理」として考えるStrategyパターンは同一のインターフェイスでメソッドを呼び出す事が必要なので、

本文3の例のコードの場合、同一のインターフェイスを持っていないのでStrategyとしては利用できないでしょう。

上記の「interface&骨格実装クラスを定義するパターン」で実装すれば、Strategyとしても使えます。

「interface&骨格実装クラスを定義するパターン」と「Facade(BisinessDelegate)」を組み合わせる方法もありますよね。

大規模なシステム開発の場合には、有効なアプローチかもしれません。

4. 「定数クラス(&パッケージ)」例の、より良い実装

この例は多くの方が見た事があるのではないでしょうか・・・。

どこでも使うような定数は、Stringとして実装するのではなく、

Java1.5以降ならenum、Java1.4以前であればタイプセーフenumパターンで実装した方が危険性がかなり低くなります。

タイプセーフenumパターンの例を上げてみます。

(簡単な例なので、コメントは略します)

public class Code implements Comparable, Serializable{
    static final long serialVersionUID = 5493754983251974328L;
    private int id;
    private String name;
    public static final Code AA0000 = new Code(1, "AA0000");
    public static final Code AA0001 = new Code(2, "AA0001");
    public static final Code AA0002 = new Code(3, "AA0002");
    public static final Code AA0003 = new Code(4, "AA0003");
    private static final Map codeMap = new HashMap();

    protected Code(int id, String name) {
        this.id = id;
        this.name = name;
    }

    protected static void createEnumMap(Class clazz, Map enumMap){
        for (Iterator iter = Arrays.asList(clazz.getFields()).iterator(); iter.hasNext();) {
            Field field = (Field) iter.next();
            try {
                enumMap.put(field.getName(), field.get(null));
            } catch (Exception e) {
                continue;
            }
        }
    }

    public static Map values(){
        if (codeMap.isEmpty()) createEnumMap(Code.class, codeMap);
        return Collections.unmodifiableMap(codeMap);
    }
    public String toString() {
        return name;
    }
    public String getName() {
        return name;
    }
    public int getId() {
        return id;
    }
    public boolean equals(Object o) {
        return this.id == ((Code)o).id;
    }
    public int compareTo(Object o) {
        return this.id - ((Code)o).id;
    }
}

このようなタイプセーフenumクラスに振る舞いを持たせると、Strategyとしても利用できます。

JavaBeanのプロパティに振る舞いを実装したインスタンスを保持させ、

処理をそのインスタンスに委譲したりすると、if文が格段に減ります。

Java1.5以降のenumも振る舞いを持たせられますし、タイプセーフenumパターンより簡潔に記述できて良い感じです。

とりあえず適当に考え付いた、より良い実装を考えてみましたが

何せ、プログラミング暦2年未満なものですから、間違ってる事があるかもしれません。

ここはおかしいとか、ここはもっとこうした方がいいよー、などありましたら、遠慮なくご指摘頂ければと思います。

このエントリーをはてなブックマークに追加
comments powered by Disqus