2011年9月29日木曜日

リファクタリング本の 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以上とかで設定しても警告が大量発生して、結局「今回は諦めようぜ」って感じになる。

人生って厳しいよね。

0 件のコメント:

コメントを投稿