リファクタリング本の Cyclomatic complexity

『リファクタリング』 を読んだ人なら、以下のコードに見覚えがあるかもしれない。第一章「最初の例」でのリファクタリング実演のために用意された架空の「長すぎるメソッド（Long method）」で、このメソッドを含む一群のコードを改善する過程で、リファクタ術が披露される。

まあ、実演のためのサンプルだから、本気でバカ長いメソッドを掲載するわけにはいかないだろうけど、それでも全然簡潔ではないし見た目も悪い。

自分なら普通、こういうのを製品コードに残したりはしないけど、それは単に自分にとって綺麗とか汚いといった審美的な感覚だけではなくて、他の開発者のミスリーディングを引き起こしたり生産性を下げたりといった、つまり他人様に迷惑をかける真似はすべきでないという、割と真面目というか普通な社会人的常識からでもあったりする。

  public String statement() {
      double totalAmount = 0;
      int frequentRenterPoints = 0;
      Enumeration rentals = _rentals .elements();
      String result = "Rental Record for " + getName() + "\n";
      while (rentals.hasMoreElements()) {
         double thisAmount = 0;
         Rental each = (Rental) rentals.nextElement();
         
         switch (each.getMovie().getPriceCode()) {
         case Movie.REGULAR:
            thisAmount += 2;
            if (each.getDaysRented() > 2)
               thisAmount += (each.getDaysRented() - 2) * 1.5;
            break;
         case Movie.NEW_RELEASE:
            thisAmount += each.getDaysRented() * 3;
            break;
         case Movie.CHILDRENS:
            thisAmount += 1.5;
            if (each.getDaysRented() > 3)
               thisAmount += (each.getDaysRented()  - 3) * 1.5;
            break;
         }
         frequentRenterPoints ++;
         if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) &&
               each.getDaysRented() > 1) frequentRenterPoints ++;
         
         result += "\t" + each.getMovie().getTitle() + "\t" +
         String.valueOf(thisAmount) + "\n";
         totalAmount += thisAmount;
      }
      result += "Amount owed is " + String.valueOf(totalAmount) + "\n";
      result += "You earned " + String.valueOf(frequentRenterPoints) +
      " frequent renter points";
      
      return result;
   }

ところで、このコードの扱いにくさを「計測」するとどうなるか。よく使われている尺度としては、Cyclomatic Complexity があるが、どんなもんだろう。

ちなみに、CheckStyle の解説では、

• 1-4: considered good
• 5-7: ok
• 8-10: consider re-factoring
• 11+: re-factor now!

とある。

で、実際に測ってみると出てきた複雑度は 9 だった。"consider re-factoring"という事になるから、本の中での実演のネタとしては、正に丁度良かったという事になる。

さらに本に従って最後の段階までリファクタを施したコードの CyclomaticComplexity を測ってみると、結局 2 にまで低減していた。さすがにこれくらいまでやると、変な引け目も後ろめたさもなくソース・コードを共有できる。

で、せっかくある程度客観的にコードの良し悪しがでるのだから、自分だけじゃなくチームにも、簡潔で扱いやすいコードを書く方向で前向きに頑張ってほしいと思うのだけど、いろんな人が集まる現実のプロジェクトではそうも言ってられない場合も多い。

特に、すでにある程度コードが書き貯まったプロジェクトで、途中で CyclomaticComplexity チェックを適用したりなんかすると、「11+」どころか、妥協して15以上とかで設定しても警告が大量発生して、結局「今回は諦めようぜ」って感じになる。

人生って厳しいよね。

依存 jar を変更したらどうなるか

あるアプリケーションがビルド時に参照していた ライブラリの jar を変更して、そのアプリケーション再コンパイルせずに実行したらどうなるか。

====
以下のようなライブラリ・コードを書き、コンパイルして foo.jar に丸めておく。

package p1;

public class Foo {
  public String bar() {
    return "hello";
  }
}

で、このライブラリを使う以下のようなクライアントコードを書いて、-cp に foo.jar を指定してコンパイルする。

package p2;

import p1.Foo;

public class Client {
  public static void main(String[] args) {
    System.out.println(new Foo().bar());
  }
}

コンパイルされたクラスを java コマンドで -cp に foo.jar を指定して実行すると、標準出力に hello と出力される。

さて、ここで クラス Foo を変更して、つまり foo.jar の中身を変えて、再コンパイルせずに再度 Client#main() を実行したらどうなるか。

(1) メソッドの中身を変えてみる

  public String bar() {
    return "good-bye";
  }

問題無し。標準出力に good-bye と表示される。

(2) メソッドの名前を変えてみる

  public String Bar() {
    return "hello";
  }

メソッド p1.Foo.bar()Ljava/lang/String が見当たらないって事で、NoSuchMethodError が送出される。リフレクションのコーディングでよく catch したりするNoSuchMethodExceptionではなく、NoSuchMethodError が投げられてきた。

(3) メソッドの引数を変えてみる

  public String Bar(String s) {
    return "hello";
  }

これは (2) と同じ

(4) メソッドの戻り値を変えてみる

  public Object Bar() {
    return "hello";
  }

これも (2) と同じ。戻り値も識別される。

(5) throws を追加してみる

  public String bar() throws IOException {
    throw new IOException("test");
  }

これは意外にも普通に実行されて、IOExceptionが送出されてスタックトレースされる。意外というのは、これを再コンパイルしようとすると、コンパイルエラーが出るからで、Client#main() に throws を追加するか、bar() を try-catch で囲むかしないと、コンパイルが通らない。でも、コンパイルは通らなくても実行時のメソッド呼び出しは成功する。

(6) メソッドの可視性を変えてみる

  private String bar() {
    return "hello";
  }

これは IllegalAccessError が送出される。つまり、一応 p1.Foo.bar()Ljava/lang/String の存在は識別された上で、アクセスに失敗して例外が発生した模様。

(7) クラスの可視性を変えてみる

class Foo {
…

(6) と同様だが、クラス p1.Foo の存在を識別したあとに、アクセスするところで失敗している。

(8) 定数を変えてみる

メソッドは以上のような感じで、フィールドもだいたい想像がつく。で、ここでちょっと定数を試してみる事にする。

まずライブラリコードを以下のように変える。

package p1;

public class Foo {
  public static final int C = 100;
}

次に、クライアントコードを以下の様に変える

package p2;

import p1.Foo;

public class Client {
  public static void main(String[] args) {
    System.out.println(Foo.C);
  }
}

両方コンパイルして実行すると、標準出力に100が実行される。

ここでライブラリコードを以下の様に修正し、jar を作り直す。

  public static final int C = 200; 

で、クライアントコードを再実行すると、標準出力に 200 が出力されると思いきや、実際には変更前と同じ100が出力される。
javap で見てみると、バイトコードに定数 100 が埋め込まれているのがわかる。なるほど定数はこういう扱いらしい。

public static void main(java.lang.String[]);
  Code:
   0: getstatic #2; //Field java/lang/System.out:Ljava/io/PrintStream;
   3: bipush 100
   5: invokevirtual #3; //Method java/io/PrintStream.println:(I)V
   8: return 

(9) null値 を試してみる

ライブラリ・コード を以下のように変更して、両方とも再コンパイルして実行してみる。

package p1;

public class Foo {
  public static final String C = null;
}

標準出力には、null が出力される。

ここでライブラリ・コードを、public static final String C = "a"; に変更して、クライアントを実行してみる。

(8) の結果から、null が出力される結果を類推してしまうが、実際には "a" が出力される。理由は、Java 言語仕様として null 定数として扱われないためコンパイル時には byte コードに直接書き込まれず、実行時に初めて Foo.Cの値を読むことになるかららしい。javap は以下のようになる。

public static void main(java.lang.String[]);
  Code:
   0: getstatic #2; //Field java/lang/System.out:Ljava/io/PrintStream;
   3: getstatic #3; //Field p1/Foo.C:Ljava/lang/String;
   6: invokevirtual #4; //Method java/io/PrintStream.println:(Ljava/lang/String;)V
   9: return