深いネストは悪

TL;DR

• 深いネストは悪い設計の兆候。
• 動いていればよいではなく、条件を整理し、深いネストを削減します。

具体的な例

次に示すのは実際に現場で実装されていたコードです。

処理の順序以外の箇所は改変しています。

リストから値を取り出し、顧客番号が空ではないときだけ処理を実行します。

for (FooDto fooDto: fooDtoList) {
  // 顧客番号が空ではない場合処理を行う。
  if (!CUSTOMER_NO_EMPTY.equals(fooDto.getCustomerNo())){
    List<BarDto> barList = barRepository.selectBarList(fooDto.getCustomerNo());
    // リストが空でない場合、処理を行う。
    if (barList.size() != 0) {
      for (BarDto barDto : barList) {
        // BAR番号が空でない場合処理を行う。
        if (!BAR_NO_EMPTY.equals(barDto.getBarNo())){
            // 正常処理
        } else {
            // エラー処理
        }
      }
    } else {
      // エラー処理
    }
  } else {
    // エラー処理
  }
}

次の順序で処理をしています。

1. FOOのリストをループします。
2. 顧客番号が空でない場合は処理を継続します。
3. 空の場合はエラー処理を行います。
4. 顧客番号からBARのリストを取得します。
5. リストが空でない場合、処理を継続します。
6. 空の場合はエラー処理を行います。
7. BAR番号が空でない場合は目的の処理を実施します。
8. 空の場合はエラー処理を行います。

最初の実装者など処理を深く理解できていれば、一見問題なさそうに見えます。
しかし、ネストの深さが原因で条件の関連性を見落とし、改修時にバグが発生しました。

レビューやテストで発見できるのでは？
正しいレビューやテストが行われていればの条件が必要です。
それらについては別の記事でお話しします。

ネストの削減

より見やすく理解できる処理に変更をします。

1. FOOのリストをループします。
2. 顧客番号が空の場合はエラー処理を行い、処理を終了します。
3. 顧客番号からBARのリストを取得します。
4. リストが空の場合はエラー処理を行い、処理を終了します。
5. BAR番号が空の場合はエラー処理を行い、処理を終了します。
6. 目的の処理を実施します。

具体的にコードにしてみましょう。

for (FooDto fooDto: fooDtoList) {
  // 顧客番号が空の場合はエラー処理を行い、処理を終了します。
  if (CUSTOMER_NO_EMPTY.equals(fooDto.getCustomerNo())) {
    // エラー処理
    continue;
  }
  List<BarDto> barList = barRepository.selectBarList(fooDto.getCustomerNo());
  // リストが空の場合はエラー処理を行い、処理を終了します。
  if (barList.size() == 0) {
    // エラー処理
    continue;
  }
  for (BarDto barDto : barList) {
    // BAR番号が空の場合はエラー処理を行い、処理を終了します。
    if (BAR_NO_EMPTY.equals(barDto.getBarNo())) {
      // エラー処理
      continue;
    }
    // 正常処理
  }
}

ネストが浅くなり、わかりやすくなりました。
後の改修担当もバグを混入することなく実装できるでしょう。

設計そのものの再検討

ネストが深い場合は設計を見直してみることも必要です。
たとえば、ここでは目的の処理に加えて、データの不整合についてエラー処理を実施しようとしていますが、
そもそも不整合については別の処理として実装し、ここでは正常データのみに対して処理を実施するとします。
そのような場合、次のような実装も可能です。

fooDtoList
  .stream()
  .filter(dto -> !CUSTOMER_NO_EMPTY.equals(dto.getCustomerNo()))
  .forEach(dto -> barRepository.selectBarList(dto.getCustomerNo())
          .stream()
          .filter(barDto -> !BAR_NO_EMPTY.equals(barDto.getBarNo()))
          .forEach(barDto -> // 正常処理)
  );

まとめ

深いネストを実装してしまいそうなときは次を確認してみましょう。

• ifの条件は適切か。
  裏の条件で実装できないか
• その設計は適切か。
  複数の処理を同時にしようとしていないか