こんにちは、アカウンティングサースジャパンのShrimpmanです。
Javaアプリケーションの現場で働いていると、言語が十分枯れていることもあり
レガシーなJavaアプリケーションのメンテナンスをすることは少なくないと思います。
そうすると当時のJava初学者によるちょっと現代のプログラマでは理解し難い不思議なコードや、
あわやバグを生み出す危険なコードに出会うことも少なくないかと思います。
今回はその中から特に印象に残ってるものをランキング形式で発表したいと思います。
(ほとんど当社以外の事例ですが)実際に本当にあったことなのです。
No.10 public staticな変数
public staticな変数とはそのVM内で一つのいわばグローバル変数のようなものですよね。
Connectionを様々な場所から取得したかったのでしょうか。
public static Connectionなどと宣言されていたから大変です。
connectionを取得したらすでにクローズしていたなんて問題が当然発生します。
No.9 private static (finalでない)ロガー
private static (final修飾子のない)宣言のLoggerがあっちこっちに使われていました。
これ自体は特段問題になりませんが、このクラスともすればがSerializeの対象のクラスだった場合は
当然LoggerはSerialize不可能なので例外を投げられてしまっていました。
それがあちこちで氾濫していたからたまらない。
private static Logger log = Logger.getLogger(this.getClass());
No.8 finalizeをoverride
Objectクラスのfinalizeメソッドは、そのObjectに対する参照がなくなり
ガベージコレクタにより破棄対象として回収された時に実行されますよね。
そのfinalizeメソッドをオーバーライドして処理を書いていたから困ったものです。
処理が実行されたりされなかったり、いつ呼ばれるかもわからないなんてことになっていました。
No.7 階層化した構造をつくり、ひたすらnullチェック
SomeValue value = null;
if (someClass != null
&& someClass.getValueA() != null
&& someClass.getValueA().getValueB() != null
&& someClass.getValueA().getValueB().getValueC() != null
&& someClass.getValueA().getValueB().getValueC().getValueD() != null
&& someClass.getValueA().getValueB().getValueC().getValueD().getValueE() != null
&& someClass.getValueA().getValueB().getValueC().getValueD().getValueE().getValueF() != null
&& someClass.getValueA().getValueB().getValueC().getValueD().getValueE().getValueF().getValueG() != null) {
value = someClass.getValueA().getValueB().getValueC().getValueD().getValueE().getValueF().getValueG();
}
うわぁ・・・・。
No.6 シングルトンではない謎の副作用クラス
昔はよくありましたよね、こんなクラス。今となってはほとんど見かけなくなりました。
public class SomeClass {
private Result result;
public void init() {
//何か処理 マスター読み込みとか
}
public void calculate() {
//計算処理
result = new Result(xxx);
}
public Resut getResult() {
return result;
}
}
No.5 例外を全部投げる
うーん、これはそんな見かけないですが、たまにあります。
public int extractSomethingUtilityMethod throws IOException, IllegalStateException, NumberFormatException, SomeException.... {
//何か処理
}
たくさんの種類の例外の発生を宣言されています・・。
潰されるよりマシだけど、こんなメソッドは呼びたくない・・・。
No.4 変数の無意味な使い回し（変数のスコープが無駄に広い）
SomeElement element = null;
if (valueA != null) {
someElement = new Element(valueA)
someElement.setParent(parentElement);
}
if (valueB != null) {
someElement = new Element(valueB)
someElement.setParent(parentElement);
}
if (valueC != null) {
someElement = new Element(valueC)
someElement.setParent(parentElement);
}
気持ち悪い・・・・これだけなら特に悪影響なさそうですが、
不可解なスコープにちょっとゾワッとしますよね。
No.3 例外の抹殺
たまに見かけて発狂するコード
try {
//色々処理
} catch (Exception e) {
}
例外キャッチしてるけど・・、エーーーーー！？ログ吐かないの！？っていう・・・。
No.2 forループからのラベルによる離脱
Java8が中心となった今では登場シーンが少なくなったforループですが、
レガシーなプログラムではスタックしない効率の良いループ処理としてあらゆる場所で便利に使われていました。
しかしforループを便利に使いすぎるあまり、forループを二重三重にもネストさせ、さらにループからの離脱に
ラベルを使う賢者が現れました。
loopA: for (int i = 0; i < arrayA.length; ++i) {
for (int j = 0; j < arrayB.length; ++i) {
//なんか処理
if (xxx) {
break loopA;
}
}
}
このくらいは序の口ですが、これを多重化させたらいつどこまでループ抜けたか全然わかりません。
No.1 else ifは使えないコーディングルールを（自ら？）課したHARDモード
タイトルの通りと言いたいところなのですが、ちょっと想像し難いと思いますので下に簡単に例を書きます。
if (bigDecimalValue.compareTo(BigDecimal.valueOf(100000L) < 0)) {
//処理
} else {
if (bidDecimalValue.compareTo(BigDecimal.valueOf(200000L) < 0) {
//処理
} else {
if (bigDecimalValue.compareTo(BigDecimal.valueOf(300000L) < 0 ) {
//処理
} else {
これが延々と続き、画面のかなた右へ消えていく・・・
}
}
}
初めてこれを見た人は、この処理が何をしているかはわからないけど条件分岐を
else if無しでコーディングしなければならない会社に就職してしまったのかと発狂すると思います。
ちなみに筆者が遭遇したこのif文は、どんどんネストしていき画面のかなた右へと消えていきました。
なお、ASaaSではこの数年で技術的リードエンジニアを増員し、若手教育にも力を入れていますので、
品質の高い製品づくりが行える体制が整っております。
ASaaSをご利用くださる全てのお客様に長く安心してご利用頂ける製品づくり、保守体制を
整えていますので、どうぞご安心ください。