privateメソッドは不要、出力パラメータに御用心

書いてあることに同意。Meyer センセを始めとして「値を返すだけの function と副作用を伴う procedure は分離しる！」とむかしから言われてるアレと同根かなぁと。

で、トラバ元のエントリとは直接関係無いのだけど、このエントリ読んでて思い出したことがあるのでそれについて。

共通関数（この言い回しは良くないかな？）として static なメソッドに切り分けてるつもりで、やばいことになってるコードについてです。

public class OutputParameter {
    private List<String> privateList;

    public OutputParameter() {
        super();
        this.privateList = new ArrayList<String>();
    }
    
    public void procedureA() {
        CommonNantoka.hoge(privateList);
    }
}

public class CommonNantoka {
    public static void hoge(List<String> list) {
        if (/*何かしかの条件*/) {
            list.add("hoge");//引数で渡されたオブジェクトが更新されている！
        }
    }
}

なぜこのコードがヤバイというか読みにくくなるのか？

これをやられると、あるクラスのメソッドを追っているとき（この例だと OutputParameter#procedureA）に何時の間にかインスタンス変数の状態が更新されている、という事態が発生します。こうしたコードを読む場合、あるメソッドを追いかけるときに、そのメソッド中で使用されているすべての関数のコードを追う必要がでてきます。なぜかというと、何時どこでどのような条件である変数が更新されるかが、あるクラスのメソッドを見ただけでは不明だからです。

こうした状況下では、プログラマは何時どこでどのような条件で変数が更新されるかをすべて把握しておかなければなりません。そうしないとメソッドの振る舞いを完全に把握することが出来ないからです。あるメソッドを追いかけるプログラマは何時どこでどのような条件で変数が更新されるのかを知るために、あるメソッドから呼び出されているすべてのメソッドの中身をすべてほじくりかえし、変数が更新される条件と箇所をすべて頭の中に入れておく必要があります。

まーぶっちゃけクラスつーかカプセル化の恩恵放り出してるわけですな。なので、何もかもをも覚えながらコードを読む羽目になってしまうのは当然です。で、どーせそんなもん覚えきれないのでどっかで破滅。別にカプセル化うんぬんを持ち出さなくても、インスタンス変数が private であればプログラマはその変数はそのクラス内でしか更新されないことを「期待」するわけなので、その点だけでも害が大きいやり方なわけです。

というわけで「引数で渡されたオブジェクトの状態が更新される」という出力パラメータを多用するコードはイクナイ。C 言語だとこういう引数渡しつーかポインタ渡しは使わざるを得ないので仕方ない局面もあるけど、Java だとトラバ元のエントリにあるように内部クラス作るなり別クラス作るなりしたほうが良いと思います。

出力パラメータなコードに慣れてるとこういう風に書いてしまいがちなんですよねぇ。その場限りにおいては何かと便利だったりするので……でも、悔い改めなければならんですね。プログラミング失敗談のカテゴリのエントリを増やすときはむかしの自分に石を投げるつもりで書いている kagamihoge です。

オマケ。はてなハイクでネタにしたコードはこんな感じです。

/**
 * 中身がほげなリストを返す
 * 
 * @param list ほげList
 */
public static void getHogeList(List<String> list) {
    for (/*省略*/) {
        list.add();
    }
}

javadoc に「中身がほげなリストを返す」って書いてあって、メソッドの prefix が get なのに返り値が void 「はぁ？」と思ってコードの中を見てみると……「中身がほげなリストを返す」って、引数のオブジェクト更新して返すって意味かよ！　と仰け反ってしまいましたとさ。

出力パラメータのご利用は計画的に。

オマケその２。ちなみにもっと露骨にやばいヤツだとこんな感じ。クラスを作るということは getter/setter を Eclipse で自動生成しなければならぬーと思ってる人はこう書いてしまうみたいですね。

public class OutputParameter {
    private List<String> privateList;

    public OutputParameter() {
        super();
        this.privateList = new ArrayList<String>();
    }
    
    public List<String> getPrivateList() {
        return this.privateList;// なにそれこわい
    }

いつでもどこでも private なオブジェクトをいじり放題とは何とも便利なことですね？

kagamihogeの日記

kagamihogeの日記です。

privateメソッドは不要、出力パラメータに御用心