メソッドを読みやすく

なんとなく。 読みやすさの定義はいろいろあると思うがふと思いついたので。
例えばこのような信号を表すクラスとそれに伴う挙動をenumで定義する。

使うクラスとか(これは準備)

   static class Signal {
        boolean isBroken;
        Color color;
        public boolean color() {
            return this.color;
        }
        public boolean isRed() {
            return this.color == Color.RED;
        }
        public boolean isBlue() {
            return this.color == Color.BLUE;
        }
        public boolean isYellow() {
            return this.color == Color.YELLOW;
        }
        public boolean isBroken() {
            return this.isBroken;
        }
        static enum Color {
            RED,
            YELLOW,
            BLUE;
        }
    }
    
    static enum Do {
        STOP,
        ATTENTION,
        GO;
    }

本題

あえての 読みづらいパターン

①変数への再代入をしている
②ネストが発生している
の2点があることで若干可読性に欠ける
以下のメソッドはこんな風に読める。
信号が壊れていないなら、
赤だったら止まるようにする。
赤でなくて黄色なら注意するようにする。
赤でなくて黄色でもなく、青なら進むようにする。
赤青黄のいずれでもないなら異常です。
赤青黄の色に従って動きます。

   Do doing(Signal signal) {
        Do doing = Do.STOP;
        //信号が壊れているならその時点で止まるでいいはず
        if (!signal.isBroken) {
            //赤でないなら~黄色でないなら~とかいう思考は普通しないはず
            if (signal.isRed()) {
                doing = Do.STOP;
            } else if (signal.isYellow()) {
                doing = Do.ATTENTION;
            } else if (signal.isBlue()) {
                doing = Do.GO;
            } else {
                throw new IllegalStateException();
            }
        } 
        return doing;
    }

ちょっと改善

①早期リターンを使用して信号が壊れていたらその時点でreturn
②条件ごとにreturnをすることでネストをなくす
の2点を実施することで、読みやすくなる。
以下のメソッドはこんな風に読める。
信号が壊れているなら、止まる。
信号が赤なら、止まる。
信号が黄色なら、注意する。
信号が青なら、進む。
そうじゃないならなんかおかしい。

   Do doing(Signal signal) {
        if (signal.isBroken()) return Do.STOP;
        
        if (signal.isRed()) return Do.STOP;
        if (signal.isYellow()) return Do.ATTENTION;
        if (signal.isBlue()) return Do.GO;

        throw new IllegalStateException();
    }

もうちょっと改善

enum自体にメソッドを持たせて挙動を変えさせる。
②呼び出し側ではenumの持つメソッドを呼び出すだけ。
ここまでする必要あるかわからんがenumの力をお借りして条件式をそもそもなくす。
条件式をなくすことで、コードリーディングが減る(でいいのかな?)

   static enum Color {
        RED {
            @Override
            Do doing() {
                return Do.STOP;
            }
        },
        YELLOW {
            @Override
            Do doing() {
                return Do.ATTENTION;
            }
        },
        BLUE {
            @Override
            Do doing() {
                return Do.GO;
            }
        };
        abstract Do doing();
    }

    Do doing(Signal signal) {
        if (signal.isBroken()) return Do.STOP;
        
        return signal.color().doing();
    }

まぁcolor().doing()ってのは変だけど。こんな感じかね。