今まで出会ったBad Coding.(java6)

某プロジェクトでみた未来へ引き継ぎたくないコーディング例を書いていく。
できれば二度と会うことのないよう祈願して厄落とし的に書いていきたい…
java6で開発している世界の話。

ここに記載したコードは、某プロジェクトで見た邪悪な既存コードをなんとなく再現したものです。
すでにバッドノウハウと化したコード、意味をなさないコード、バグ、頭おかしいやつなど混在してます。

インスタンスの再利用

public class HogeBean {
  private String key = "";
  private val = 0;
}

public class LogicClass{
  public void bizLogic(List inputList){
    HogeBean hoge = new HogeBean();
    for(Object input: inputList){
      init(hoge); //すでに存在するインスタンスの各フィールドを初期化
      hoge.setKey(input.getXXX());
      hoge.setVal(input.getYYY());
      // hogeを処理する
    }
  }

  private void init(HogeBean b){
    b.setKey("");
    b.setVal(0);
  }
}

太古の昔であれば、newに時間がかかるからー、という性能のためにこのアプローチあったと思うんだけど、今となっては可読性を殺しにかかってたし意図がわからないコードになってるからきつい。
いまなら、普通にループでもnewしてやったほうがいいと思うの。

過剰なリテラルの忌避

final static String BLANK_STRING = "";
if(BLANK_STRING.equals(hoge)){
  //処理
}
const int INT_ZERO = 0;
if(fooArray.size() == INT_ZERO){
  //処理
}

文字列に対して""とか、sizeに対して0を比較くらいであれば、リテラルで指定すればいいのに。いいのに。

過剰なリテラルの忌避２

const int INT_THREE = 3; //3
if(inputVal == INT_THREE){
  //処理
}

適当に名前つけたらマジックナンバーじゃなくなるわけじゃないし、コメントもジョークかとおもった。

return (条件文) ? true : false;

return (条件文) ;でええんやで。

int am7 = 070000

※なぜか数値で時刻を表現してた実装。

int am7  = 070000; // 7時
int pm18 = 180000; //18時

多分桁合わせるために、0つけたんだろうけど、
頭に0つけた数値リテラルなんて８進数になるに決まっとる…
そもそもなんで時刻をint値で表現していたのか…

ProjectNameUtil.isNull(String s)

public boolean isNull(String s){
  return s == null || "".equals(s);
}

名前と挙動が一致してない。完全にトラップ実装。
多分長さ0の文字列はnullと一般的に言われていると思ってるやつが書いた。
ちなみにプロジェクトでは普通にStringUtilsがインポートされてるので、車輪の再生産な上に悪質。
こういう実装が既存コードの奥にそれなりに存在していたので、ソースリーディング時は奥深くまで読まなければならずレビュアとしてはストレスフル。

まとめ

厄落としとして書いてみた、
今回の開発は元から存在した既存コードが今回の記事のような邪悪さをもつコードだったので、今後はこのような負債と関わり合いにならないように生きたい。