Based on a recent discussion I had with a friend of mine, I am curious to know what other people think.

The discussion revolves around the use of defensive programming techniques, as implicitly discussed in my recent article. I am going to discuss two issues.

public void doSomething(SomeClass pClass) {
    ... // Do some things
    pClass.executeSomeFunction();
    ... // Do some other things
  }

Consider what will happen if pClass is null. Code like this will cause the following scenario to happen at runtime:

waldo@waldonbl Play $ java app
  Exception: null
  java.lang.NullPointerException
        at app.doSomething(app.java:17)
        at app.main(app.java:31)

If you have access to the source code, I guess this specific example is not too bad to resolve. However, if you do not have access to the source code or the pClass class was used as pClass.getSomeObject().executeSomeFunction(), then how would you know what was null (in the first example), and whether it was pClass or getSomeObject() that was null (in the last example).

I prefer writing the above piece of code as follows:

public void doSomething(SomeClass pClass) {
    ... // Do some things
    if (pClass == null)
      throw new InternalErrorException("pClass was null");

    pClass.executeSomeFunction();
    ... // Do some other things
  }

This code will at least tell me exactly where and what the problem is without me needing to look at the source code. And if it was the nested object scenario, I would know which object was null without adding debug code.

Secondly, which piece of code is the better (i.e. safer):

vConMgr = ConnectionManager.getInstance();

 try { 
   vCon = vConMgr.getConnection(pDatabaseName);

    try {                
      vStmt = vCon.createStatement();
      if (vStmt == null)
        throw new Exception("Internal Error: vStmt is null in XX.f()");

      try { 
       vRes = vStmt.executeQuery("SELECT * FROM some_table");

       try {  
          while (vRes.next()) {
            do_something();
            vCount++;
          }
       }
       finally {
         vRes.close();
       }
      }
      finally {
       vStmt.close();
      }
    }
    finally {
      vConMgr.freeConnection(pDatabaseName,vCon);
    }
 }
 finally {
  vConMgr.release();
 }

or

vConMgr = ConnectionManager.getInstance();

 try { 
   vCon = vConMgr.getConnection(pDatabaseName);

    try {                
      vStmt = vCon.createStatement();
      vRes = vStmt.executeQuery("SELECT * FROM some_table");
      while (vRes.next()) {
        do_something();
        vCount++;
      }
    }     
    finally {
      if (vRes != null)
        vRes.close();
      if (vStmt != null)
        vStmt.close();
      vConMgr.freeConnection(pDatabaseName,vCon);
    }
 }
 finally {
  vConMgr.release();
 }

I prefer the first, since it handles NullPointerExceptions much better and it will properly close vStmt if there was an exception after vCon.createStatement() (which the latter one will not do).

Anyone with experience in Java will immediately point out that it is redundant to protect vStmt and vRes in order to properly close them, since Java will do that for us. I know that. However I prefer to handle that the right way since Sun cannot dictate what JDBC vendors will do with their Statement and ResultSet objects internally in the JDBC driver, when the connection is released. Furthermore, even thought vStmt and vRes go out of scope at the end of the function call, it is up to the garbage collector to reclaim the memory used by those objects. By explicitly indicating I am finished with them, the garbage collector can reclaim that memory much earlier. It just is good programming practise.

I would love to hear what your opinions on this matter are. Please comment away.