m2

キミのコードが汚い理由のサンプルコードが読みにくい理由

それはオブジェクト指向じゃないから。
キミのコードが汚い理由 〓 @IT
http://www.atmarkit.co.jp/im/carc/serial/redge51/redge51.html
以下はよりクリーンだとされるコードの一部。

    private int[] gameWon = {0,0};
    (中略)
    public String getSetScore() {
        int leader = gameWon[0] > gameWon[1] ? 1 : 2;
        int leaderGames = gameWon[leader - 1];
        int opponentsGames = gameWon[leader == 1 ? 1 : 0];

汚いよ。*1
Java ならもっとふつーにオブジェクト指向してください。
以下、ポイント。

  1. テニス(プレイヤーが二人)
  2. 複雑な戦況判定
  3. 判定結果によってメッセージを変える
class Player {
    private String name;
    private int games;
    public Player(String name) {
        this.name = name;
        this.games = 0;
    }
    public String getName() {
        return name;
    }
    public int getGames() {
        return games;
    }
    public void gameWon() {
        games++;
    }
}

public class OneSetScorer {
    private Player p1, p2;
    public OneSetScorer() {
        p1 = new Player("Kent");
        p2 = new Player("Derricott");
    }
    public void player1GameWon() {
        p1.gameWon();
    }
    public void player2GameWon() {
        p2.gameWon();
    }
    public String getSetScore() {
        if ( isFinished() ) {
            return 
                  getLeader().getName() + " wins the set " 
                + getLeader().getGames() + " - " 
                + getOpponent().getGames();
        }
        else if ( isTie() ) {
            return "Set is tied at " + getLeader().getGames();
        }
        else {
            return
                  getLeader().getName() + " leads " 
                + getLeader().getGames() + " - "
                + getOpponent().getGames();
        }
    }
    private Player getLeader() {
        return ( p1.getGames() >= p2.getGames() ) ? p1 
                                                  : p2;
    }
    private Player getOpponent() {
        return ( getLeader() == p1 ) ? p2 
                                     : p1;
    }
    public boolean isTie() {
        return p1.getGames() == p2.getGames();
    }
    public boolean isFinished() {
        if ( getLeader().getGames() == 7 ) {
            return true;
        }
        if (
            getLeader().getGames() == 6
            && 
            getOpponent().getGames() < 5
        ) {
            return true;
        }
        return false;
    }

    public static void main(String[] args) {
        OneSetScorer scorer = new OneSetScorer();
        int n = 0;
        do {
            if ( Math.random() < 0.5 ) {
                scorer.player1GameWon();
            }
            else {
                scorer.player2GameWon();
            }
            System.out.println( scorer.getSetScore() );
            n++;
        } while ( ! scorer.isFinished() && n < 13 );
    }
}
  • -

よりクリーンとされるコード(リスト2)にバグがあるらしい。

リスト1とリスト2が例に挙がっていて、リスト2のほうがいいよ、っていうような感じの記事なんだが、

リスト2のほうはバグがある。引き分けのはずなのに"Player2 leads"という文字列が返ってしまうのだ。


バグを筆者が見逃したのは、リスト2が読みにくくて汚いコードだからではないか。

全く同感。
僕のコードはリスト1の方をリファクタリングしたもので、リスト2はほとんど見る気がしない見ていなかったから気が付かなかった。

  • -

Java にも printf 相当の機能が追加されたことを忘れてた。国際化を考えるならこっちのほうがいいだろう。

    public String getSetScore() {
        if ( isFinished() ) {
            return String.format(
                    "%1s wins the set %2d - %3d",
                    getLeader().getName(),
                    getLeader().getGames(),
                    getOpponent().getGames()
            );
        }
        else if ( isTie() ) {
            return String.format(
                    "Set is tied at %1d",
                    getLeader().getGames()
            );
        }
        else {
            return String.format(
                    "%1s leads %2d - %3d",
                    getLeader().getName(),
                    getLeader().getGames(),
                    getOpponent().getGames()
            );
        }
    }

*1:これよりもリスト1の方が幼稚だけどもよっぽど素直なコードに見える。