安易なフラグとswitchを撲滅しよう

今日コードいじってたら思い出したので

悪いコード

public class C{
    public static final int flg0 = 0, flg1 = 1, flg2 = 2, flg3 = 3;

    public void method(int flg){
        switch(flg){
        case flg0:
        //hoge
        break;
    }
}

このコードには問題点があるわけです。

1. フラグがint型(new C().method(0)っていう書き方をしてもOKになってしまう。本当はフラグを入れないといけないのに)
2. switch(フラグが増えるほど、専用の処理をmethodに長々と書かないといけない。しかも同一スコープなので容易にスパゲティに)

フラグがint型

Javaならenumに置き換えましょう。
オブジェクト指向がサポートされている言語なら、数値の代わりに空のオブジェクトを入れましょう。
そうする事で、メソッドの引数に入れるべき値が何であるかが明確になります。少なくともint型よりは。

switch

フラグが数個ならswitchでもいいと思いますが、以下の状況では不適切です。

1. フラグが大量にある(約4個以上になるのならswitchは不適切だと思います)
2. 今はフラグが少ないが、のちのち増える事が予見出来る
3. 分岐処理のコードがそこそこ長め

そこで、switchの代わりにオブジェクトに処理を書きます。
Javaならenumに抽象メソッドを作ると便利です。

つまりこうしろ

enum Flg{
    flg0{
        public void method(){}
    },
    flg1{
        public void method(){}
    };

    public abstract void method();
}

public class C{
    public void method(Flg flg){
        flg.method();
    }

これで引数に間違った値を入れる事も、switchでスパゲティコードをつくりこんでしまうこともないはずです。
また、状態が増えてもC.methodを修正する可能性が少なくなります。

状態遷移が決まってるなら

enum Flg{
    flg0{
        public Flg method(){return flg1;}//flg1に状態遷移
    },
    flg1{
        public Flg method(){return flg0;}//flg0に状態遷移
    };

    public abstract Flg method();
}

public class C{
    private Flg flg;

    public C(Flg f){ flg = f; }

    public void method(){
        flg = flg.method();//とある状態のメソッドを実行した後、次の状態へ遷移する
    }
}

こういうのもアリかもしれません。
状態遷移をフラグ側が受け持てば、C.methodはさらに修正する手間が減るでしょう。