0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 3 years have passed since last update.

小規模改修のついでに既存のコードの問題点を修正する

Posted at

そもそもの経緯

担当となったシステムのコーディングがヤバかった。
しかし、大規模な改修案件は降りてこない。
ならば、小規模案件のついでに少しづつ直してしまおうという流れ。

現状のコーディング

例えば、検索のビジネスクラスは以下の構成になってます。
1.バリデーション
2.検索ロジックの実行
3.画面表示項目の編集

これだけ見ると、(多分)普通の構成だが、実際のコーディングは以下のような感じ。

// バリデーション
// nullチェック
if(joken1 == null ||
   joken2 == null ||
   joken3 == null ||
   ...               ) {
  error.add("エラー");
  return false;
}
// 空文字チェック
if(joken1 != null &&
   joken2 != null &&
   joken3 != null &&
   ...              ) {
  if(joken1 == "" ||
     joken2 == "" ||
     ...            ) {
  error.add("エラー");
  return false;
  }
}

// オブジェクトの生成
ObInfo obInfo = new ObInfo();
ObResult obResult = new ObResult();
int kensakuResult = 0;

// 検索条件の判定
if (joken == CONST.JOKEN1 ||
    joken == CONST.JOKEN2 ||
    joken == CONST.JOKEN3 ||
    ...                      ){
// 検索実行
  kensakuResult = obInfo.getKensakuResult(joken1, joken2, ... , obResult);
  if(kensakuResult == 0) {
    error.add("エラー");
    return false;
  }
}

// 画面表示項目の編集
ObKensakuInfoBean obKensakuInfoBean = new ObKensakuInfoBean();
editKensakuInfo(obKensakuInfoBean, kensakuResult);

// ワーニング
if (kensakuResult == 2){
  error.add("ワーニング");
} else if (kensakuResult == 3){
  error.add("ワーニング");
} else if (kensakuResult == 4){
  ...
}

return true;

以上が検索実行時に最初に呼び出されるビジネスロジックの概要です。
実際にはより多くのバリデーションがあり、項目編集があり、編集コメントがあります。
この編集コメントが厄介で、ただでさえ見づらいコードをより見づらくしています。

/* 20XX ○○案件 START */
// ObInfo obInfo = new ObInfo();
/* 20XX ○○案件 END */
/* 20XX ××案件 START */
// ObInfo obInfo 
/* 20XX ××案件 END */
   obInfo
/* 20XX △△案件 START */
//            = obGetInfo.getInfo(obInfo, obAuthInfo, obCode);
/* 20XX △△案件 END */
/* 20XX □□案件 START */
              = obGetInfo.getInfo(obINfo);
/* 20XX □□案件 END */

みたいなかんじです。
ここ一か所なら判別できますが、これがコード全体にあるので、全体のコードの把握が難しくなっています。
現在ではSVNでソース管理を行っているため、コメントは必要ないのですが、慣習としてコメントが残されている状態です。

問題点

このコードでも動いてはいます。おそらく、最初は単純な条件で十分な要件だったんだと思います。しかし、徐々に複雑な条件が必要とされてしまった。
そして、このコードは(なぜか)条件が追加されることを考慮されていません。
インプットや、if条件、バリデーションが全てビジネスロジック内部で記載されています。
そのため、改修が入るたびにビジネスロジック内部の条件・バリデーションを修正する必要がありました。
当然、改修箇所の全画面分です。
しかも、ビジネスロジックによって別クラスに条件を記載していたり、ビジネスロジックに直接かかれていたり、メソッドで記載していたりと仕様がバラバラでした。
つまり、改修箇所ごとに修正が入るクラスがバラバラということです。

対応

本来なら、バリデーションクラスを作成してそこでバリデーションを実装するのがベストかと思います。
しかし、小規模案件での作成かつ、修正ステップ数によって工数が決まるため、根本からの修正は難しいです。
なので、今回は主に検索条件の修正を行いました。

対応方法

検索条件ですが、あらかじめ決まった定数を条件として設定しているため、それらを配列にして配列内に条件と一致するかを新たに条件として追加しました。

if(CONST.JOKENS.contain(joken)){
 kensakuResult = ...
}

といった感じです。
本来は一律でまとめたかったのですが、そうすると膨大な退行テストが必要になってしまいます。(今までの条件で正常に動作することのテストが必要になる)
単純な量の問題なら、結合テストは難しくとも、単体テストだけならJUnitなどで出来るでしょう。(既存の条件に対応するJUnitのテストケースがあれば、ですが)

残念ながら、JUnitもなければローカル環境すらないので、退行テストを実施するのはあきらめました。
なので、今回は新規で追加した条件のみリスト化して対応しています。

if(joken == CONST.JOKEN ||
   ...
   CONST.JOKENS.contain(joken)){
  ...
}

という具合です。

根本原因

そもそもの原因は、立ち上げ時に詳細設計を怠ったことが始まりかと思います。
基本設計しか明確にドキュメント化されておらず、画面表示が仕様通りになるように各自が実装してしまった。
それによってクラスの動作が指定されず、クラスごとの仕様がバラバラになったのかと。
改修時のために、実装を元に詳細設計を作成するのも有りだと思いますが、そんな工数は降りないです。
それで、こういった細かい修正で対処するしかなかったわけです。

総括

設計・コーディング規約はちゃんとしよう。
運用・保守フェーズに、クラス追加するような改修の工数は降りないから。

0
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?