ResultSetをcloseする時はNULL判定すべし

気になるソースを見かけた。具体的にはこんな感じ。

try {
  Connection conn = DriverManager.getConnection(URL, USER, PASSWORD);

  String sql = "SELECT * FROM SOMETABLE WHERE ID = ?";

  PreparedStatement ps = conn.prepareStatement(sql);

  ps.setString(1, "foo");

  ResultSet rs = stmt.executeQuery();

  if (rs != null) {
    while (rs.next()) {
      // データ取得
    }
  }
} catch (SQLException e) {
  e.printStackTrace();
} finally {
  rs.close();
  ps.close();
  conn.close();
}

このソースのどこがいけないか?
作者の意図としてはエラーが発生してもResultSet、PreparedStatement、Connectionは必ずcloseしたい考えなのだろう。
だが、何らかの理由でDBにつながらない/テーブルにアクセスできない/データが取れない事態が発生しても、finallyは必ず実行される。
この場合、ResultSet、PreparedStatement、ConnectionはNULLの可能性があり、NULL判定を入れないとNullPointerExceptionで落ちる。(Web系だと最悪画面にStackTraceが表示されてしまう。)
finallyの中はこう書くべき。

} finally {
  if (rs != null) {
    rs.close();
  }
  if (ps != null) {
    ps.close();
  }
  if (conn != null) {
    conn.close();
  }
}

最近セキュアプログラミングという言葉を良く聞くが、少なくとも防御的プログラミングは心がけよう。

Effective Java 第2版 (The Java Series)

Effective Java 第2版 (The Java Series)