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

なんと前回の続きが出てきましたよ。
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 文という「不吉な匂い」があります。
その辺の解決は、たぶん次回。
・・・と、終わろうと思ったのですが、コード例があまりにもアレなので、そこはそれぞれに置き換えて読んで下さい、とだけ。
金融商品の違いやケータイのキャリアの違いなど、様々な局面で処理の分岐ってありますよね。

広告
  1. トラックバックはまだありません。

コメントを残す

以下に詳細を記入するか、アイコンをクリックしてログインしてください。

WordPress.com ロゴ

WordPress.com アカウントを使ってコメントしています。 ログアウト / 変更 )

Twitter 画像

Twitter アカウントを使ってコメントしています。 ログアウト / 変更 )

Facebook の写真

Facebook アカウントを使ってコメントしています。 ログアウト / 変更 )

Google+ フォト

Google+ アカウントを使ってコメントしています。 ログアウト / 変更 )

%s と連携中

%d人のブロガーが「いいね」をつけました。