Home > IT > Java言語で学ぶリファクタリング入門 #1 & #7

Java言語で学ぶリファクタリング入門 #1 & #7

  • 2008-12-02 (Tue)
  • IT

なんと前回の続きが出てきましたよ。

Java言語で学ぶリファクタリング入門

今日は第 1 章と第 7 章の話題で。
まず第 1 章。

シンボリック定数によるマジックナンバーの置き換え
理由1 マジックナンバーは意味が分かりにくい
理由2 マジックナンバーは修正しにくい

そろそろ使っている方も少ないと思うのですが、たまに出くわすマッジックナンバー
例えば、こういう場合、

MysidiaUtil.getShunmiFactorFrom(1);

1 って何よ?ということです。
仮のコードもいいところな MysidiaUtil クラスで説明すると、

 public static ShunmiFactor getShunmiFactorFrom(int type){
  switch (type) {
  case 1:
   //execute WarlockReport process
  case 2:
   //execute Diel process
  case 3:
   //execute Elio process
  }
  ...
 }

上の switch 文の case 1 ~ 3 もマジックナンバー。
これを単純に定数に置き換えるのが第 1 章の「シンボリック定数によるマジックナンバーの置き換え」。
まず単純に定数を作って、

 public static final int TYPE_WARLOCKREPORT = 1;
 public static final int TYPE_DIEL = 2;
 public static final int TYPE_ELIO = 3;

switch 文のコードを書き換え、

 public ShunmiFactor getShunmiFactorFrom(int type){
  switch (type) {
  case TYPE_WARLOCKREPORT:
   //execute WarlockReport process
  case TYPE_DIEL:
   //execute Diel process
  case TYPE_ELIO:
   //execute Elio process
  }
  ...
 }

呼び出す方も書き換えます。

 MysidiaUtil.getShunmiFactorFrom(TYPE_WARLOCKREPORT);

これで意味不明なマジックナンバーは消えました。
しかし・・・

引数が int である限り、いくらでもこういう書き方が可能で、

 MysidiaUtil.getShunmiFactorFrom(666);

まったく意味の異なる定数を渡すことができてしまいます(int コード値の混同)。

 public static final int TYPE_B = 1;
 public static final int TYPE_SHINNOJI = 65536;
 ...
 MysidiaUtil.getShunmiFactorFrom(TYPE_B);

TYPE_B を渡してシュンミ因子が返ってきても困るのですが、返ってきてしまうんですね。
また、TYPE_SHINNOJI だとシステムダウンするかもしれません。
そこで第 7 章。

クラスによるタイプコードの置き換え
基本型を使ったタイプコードには、
 ・異常な値になるかもしれない
 ・他のタイプコードと混同するかもしれない
という問題点がありました。

666 や TYPE_B の例がそれです。
そこで、タイプコードをクラスで置き換えます。
この辺については『Effective Java プログラミング言語ガイド』の 97 ページからが詳しいのですが、

Effective Java プログラミング言語ガイド

Java 5.0 からは、これを文法的に実現した enum があります(私は 1.4 を触った後、リュートレイクに流された)。
なので、ここは一気に enum に置き換えてしまいます。

 public enum Mysidian{
  WARLOCKREPORT(1),
  DIEL(2),
  ELIO(3);
  private final int type;
  private Mysidian(int type){
   this.type = type;
  }
  public int getType(){
   return this.type;
  }  
 }

そもそも定数のネーミングが悪かったのですが、ミシディアの人(Mysidian)ということで enum を作りました。
次に問題の getShunmiFactorFrom メソッドの引数を int から Mysidian に変更します。

 public static ShunmiFactor getShunmiFactorFrom(Mysidian mysidian){
  switch (mysidian.getType()) {
  case TYPE_WARLOCKREPORT:
   //execute WarlockReport process
  case TYPE_DIEL:
   //execute Diel process
  case TYPE_ELIO:
   //execute Elio process
  }
  ...
 }

呼び出す方は引数に Mysidian.WARLOCKREPORT を渡すように変更。

 MysidiaUtil.getShunmiFactorFrom(Mysidian.WARLOCKREPORT);

1 や TYPE_WARLOCKREPORT 渡すより、コードが読みやすくなりました。
コメントを付けるとすると、

 //ミシディアの人・ワーロックリポートからシュンミ因子を取得する

ですが、コードだけでも十分に理解可能でしょう。
当然 666 というマジックナンバーや定数 TYPE_B も渡せないので、タイプセーフということにもなります。

しかし、上の例には switch 文という「不吉な匂い」があります。
その辺の解決は、たぶん次回。


・・・と、終わろうと思ったのですが、コード例があまりにもアレなので、そこはそれぞれに置き換えて読んで下さい、とだけ。
金融商品の違いやケータイのキャリアの違いなど、様々な局面で処理の分岐ってありますよね。

Home > IT > Java言語で学ぶリファクタリング入門 #1 & #7

Page Top