m2

「キミのコードが汚い理由」のサンプルコードの最も汚い部分

前提として、私のような凡庸なサラリーマンプログラマーの立場で書いています。


キミのコードが汚い理由 〓 @IT
http://www.atmarkit.co.jp/im/carc/serial/redge51/redge51.html
はてぶで、コメントやエントリーを一通り読んだのだけども、

2007年01月12日 sawat programming 2者のゲーム数を配列で持つことに激しく違和感を感じる。

という感想はこの方だけでした。

もう一度引用するけど、以下はよりクリーンだとされるコードの一部。

    private int[] gameWon = {0,0};
    public void gameWon(int player) {
        gamesWon[player-1]++;
    }

このコードを書いた人はきっと、サッカーでそれぞれの選手のゴール数をカウントする場合はこんなコードを書く。

    private int[][] goals = new int[2][11];
    public void goal(int team,int player) {
        goals[team-1][player-1]++;
    }

goal メソッドで選手の指定を背番号で行いたいとすると、もう地獄(これは極端な例です)。

    public void goal(int team,int playerNumber) {
        if (team==1) {
            switch (playerNumber) {
                case 10: goals[0][0]++; break;
                   :
            }
        } else {
            switch (playerNumber) {
                case 23: goals[1][0]++; break;
                   :
            }
        }
    }

なにが言いたいかというと、データの持ち方を配列にするとデータのアクセスに必ず配列の添え字の計算が含まれるということです。引用したコードの中にも gamesWon[player-1]++; での「-1」のように固定の数字が何度も出てきます。
このような数字はマジックナンバーと呼ばれ、プログラム中に含まれていると著しく可読性が損なわれます。
件のコードも配列でなくすだけでかなり読みやすくなります。
before:

    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];

after:

    private int player1Games = 0;
    private int player2Games = 0;

    public String getSetScore() {
        String leadersName;
        int leadersGames;
        int opponentsGames;
        if ( player1Games >= player2Games ) {
            leadersName    = "Player1";
            leadersGames   = player1Games;
            opponentsGames = player2Games;
        } else {
            leadersName    = "Player2";
            leadersGames   = player2Games;
            opponentsGames = player1Games;
        }

プログラミングに精通した方なら後者は野暮ったく感じるものですが、凡庸なプログラマはこれでいいのです。
私たちは天才ではなく凡庸なプログラマです。凡庸なプログラマは仕様書の通りにプログラムを書けばいのです。(幸いなことにOOP言語ではそれが容易に実現できます。)できたプログラムが効率の悪いものなら仕様を最適化してください。そうすれば、「クリーン」では無いかもしれませんが、「素直」なコードになることでしょう。

  • -

気になるコメントがあったので追記

2007年01月12日 MIZ なるほど、汚い理由はわかった。で、綺麗なコードを書いたらどんな時間的・金銭的メリットがあるの?コードの再利用性が高くなるだけ?

「クリーン」だとか「素直」だとかは、使い捨てのプログラムであったりコードを誰にも見せない予定であったりすれば全く気にする必要はありません。
しかし一般的なサラリーマンプログラマが書くコードは一般的に(十)数年に渡って保守・運用・機能追加・不具合修正されるものがほとんどであろうと思われます*1。そしてその保守・運用・機能追加・不具合修正を担当する人はそのコードを書いた人ではないことがほとんどです。同じ人であったとしても、数年前に書いたコードの事なんて覚えちゃいません。
また、何度も機能追加・不具合修正されてきたプログラムでは、仕様書とコードが食い違っていることも多々あります。
素直なコードであればコードの把握がスムーズに行え、後任者が非常に楽になります。これは大きな「時間的メリット」です。時間が短ければ客の評価もUP、空いた時間に別の仕事も入れられるなど金銭的にもメリットがあります*2
逆に汚いコードであれば殺意を覚えます。
一番重要なのは「後でそのコードの面倒を看る人に気を使う」事なのだと考えています。みんなが素直なコードを書けば誰も不幸にならないのですから、特に事情が無ければそうしてください。

  • -

2007/1/25 追記
前回のようにプレイヤーがオブジェクトだともっと簡潔になります。多分これが理想形。
after2:

    private Player p1;
    private Player p2;

    public String getSetScore() {
        Player leader   = (p1.getGames()>=p2.getGames())?p1:p2;
        Player opponent = (leader==p1)?p2:p1;

やっぱ前回のは冗長すぎたかもしんない。

*1:そうではない場合もそういう心構えでいれば問題は少ない

*2:ということにしたい。実際はお客さんは時間や費用を比較する機会が無いので評価がUPするかは疑問