読者です 読者をやめる 読者になる 読者になる

HHeLiBeXの日記 正道編

日々の記憶の記録とメモ‥

無駄にCPUを使いたい処理(謎)

Java 爆弾コード

次のようなコードに遭遇したとき、一瞬、見なかったことにしようかと思った。

    private static String buildString(List codes) {
        String resStr = "";

        // 何か別の処理

        for (int j = 0; j < codes.size(); ++j) {
            if (Integer.parseInt((String) codes.get(j)) == 5001) {
                resStr += "Code-One";
            } else if (Integer.parseInt((String) codes.get(j)) == 5002) {
                resStr += "Code-Two";
            } else if (Integer.parseInt((String) codes.get(j)) == 5003) {
                resStr += "Code-Three";
            } else if (Integer.parseInt((String) codes.get(j)) == 5004) {
                resStr += "Code-Four";
            }
            resStr += ",";
        }

        // 何か別の処理

        return resStr;
    }

まずこのコードだけを見たときに突っ込みたいところ。なぜ各条件の判定ごとに同じ文字列を何度もInteger.parseIntしてるのだろうか。もっと言うならば、if文じゃなくてswitch-case文でよい場面(実際、条件を満たしたときの処理は上記コードのように単純なもの)。
あと、「Integer.parseIntしたときに解析エラーにはならないのか?」というのがあるのだが、もう一つのコードを見るとそんな疑問は吹っ飛び、別の突っ込みどころが出てくる。そのコードとは、上記処理に渡されているListを生成する処理。

    private static List getCodes() {
        /*
         * 本来は、DBから取得したデータを元にJavaオブジェクトが生成され、
         * codeはそのJavaオブジェクトのひとつのフィールドに過ぎない。
         */
        int[] codesFromDB = new int[] { 5004, 5001, 5003, 5002, 5005, 5003, 5001, 5005, 5002, 5005, };

        List res = new ArrayList();
        for (int j = 0; j < codesFromDB.length; ++j) {
            res.add(Integer.toString(codesFromDB[j]));
        }

        return res;
    }

‥何でわざわざ「int⇒String⇒int」という面倒で無駄で分かりにくいことをしているのだろう‥
もちろん、二つ目のコードは一つ目のコードのためだけに存在している。
ならば、二つ目のコードは

            res.add(Integer.toString(codesFromDB[j]));

でよいし、それを受ける一つ目のコードは

            int code = ((Integer) codes.get(j)).intValue();
            if (code == 5001) {
                resStr += "Code-One";
            } else if (code == 5002) {
                resStr += "Code-Two";
            } else if (code == 5003) {
                resStr += "Code-Three";
            } else if (code == 5004) {
                resStr += "Code-Four";
            }

でよい(もちろん、switch-case文を使ってもよい)。


一応、パフォーマンス的にも修正したほうがよいということを示すために計測してみる。
使用するコードは次のとおり。

public class Main {

    public static void main(String[] args) {
        int n = Integer.parseInt(args[0]);
        long begin = System.currentTimeMillis();
        long total1 = 0;
        long total2 = 0;
        try {
            for (int i = 0; i < n; ++i) {
                long begin1 = System.currentTimeMillis();
                List codes = getCodes();
                total1 += (System.currentTimeMillis() - begin1);
                long begin2 = System.currentTimeMillis();
                String str = buildString(codes);
                total2 += (System.currentTimeMillis() - begin2);
            }
        } finally {
            long end = System.currentTimeMillis();
            System.out.println("time: " + (end - begin) / 1000.0);
            System.out.println("total1: " + total1 / 1000.0);
            System.out.println("total2: " + total2 / 1000.0);
        }
    }

    public static List getCodes() {
// 省略(上記参照)
    }

    public static String buildString(List codes) {
// 省略(上記参照)
    }

}

これを、修正前、修正後のそれぞれのコードで実行する。ループ回数はとりあえず1000000回(百万回)で。
修正前:

time: 30.726
total1: 4.648
total2: 25.862

修正後:

time: 20.137
total1: 1.604
total2: 18.316

なんかいまいち実感のわかない秒数だけど、修正したコードによってこれだけ処理時間が減っているのは確か。