NULL POINTER eXCEPTION

[amit bhadre]

I am closing the rs1.close() in finally block even I tried to wrap in try{}catch(){} block for rs1.close().THe getConnection has return type ArrayList.But compiler do not going upto return statement.
Please help me out...

The code is as follows

package cms.history.History;
import java.io.*;
import java.sql.*;
import java.util.*;

public class History {
//ResultSet rs=null;
ResultSet rs1=null;

int candidateId=2;

public ArrayList getConnection() throws SQLException
{
ArrayList ar=new ArrayList();
ArrayList ar1=new ArrayList();

try {
Class.forName("sun.jdbc.odbc.JdbcOdbcDriver");

Connection con=DriverManager.getConnection("jdbc dbc:CMS2.0","sa","sa");
Statement stmt=con.createStatement();

//ResultSet rs1=stmt1.executeQuery("select name from CMS_EMPLOYEE ");

ResultSet rs1=stmt.executeQuery("select * from CMS_CANDIDATE_HISTORY_NOTES");// where candidate_id = "+candidateId);

while(rs1.next()) {
ar.add(rs1.getString("cand_history_id"));
ar.add(rs1.getString("cand_id"));
ar.add(rs1.getString("user_name"));
ar.add(rs1.getString("recruiter_notes"));
ar.add(rs1.getString("manager_notes"));
ar.add(rs1.getString("last_modified"));
}

/*while(rs1.next()){
ar1.add(rs1.getString("name"));
}
*/System.out.println("arrayList "+ar);
//System.out.println("arrayList "+ar1);
//return ar;

} catch (SQLException e) {
// TODO Auto-generated catch block
e.printStackTrace();
}
catch (ClassNotFoundException ex)
{

}
finally
{
rs1.close();
}
return ar;
}
public static void main(String args[]){
History h=new History();
ArrayList ary=new ArrayList();
try {
ary=h.getConnection();
System.out.println(ary);
} catch (SQLException e) {
// TODO Auto-generated catch block
e.printStackTrace();
}
}
}

Thanking you ...in advace.

[Martin Simons]

You have declared and defined rs1 outside of the try block as null.
Inside the try block you declare it again, creating a variable local to
that try block, so in the finally block, the rs1 that is seen is the one
that was defined outside of the try block, and not the one that was defined
inside of the try block. Remove the word "ResultSet" from the defining
line inside of the try block. Also, wrap your close statement inside of
an if statement i.e.

if (rs1 != null) {
  rs1.close();
}

[Periakaruppan Thiagarajan]

if (rs1 != null) {
rs1.close();
}

Whenever you execute a query and assign it to a resultset, then there is no need to check for null while closing the result set. In this case, result set will always have some value to it.

Thanks,
Pk

[Martin Simons]

Not if the execute statement fails. Or, if for whatever odd reason, the
createStatement statement fails. Or if the getConnection fails.

[ June 11, 2006: Message edited by: Martin Simons ]
[ June 12, 2006: Message edited by: Martin Simons ]

[Tony Morris]

There is no need to assign a local reference to null. Embed your try blocks, or leak - as simple as that. Yes, using null as a dynamic placeholder to determine if a reference was ever assigned has the potential to cause a resource leak, albeit how common it is.

[Jeroen T Wenting]

It is however considered good practice to initialise every method local variable to something, and in some cases it is even mandatory (if the compiler can't determine for certain that initialisation is performed later, like in a case or if-then-else construct) to prevent spurious compiler errors.

[Martin Simons]

If you try to do a close, on either a statement or a resultset or anything
else, in a finally block, and you have not initialized the variable to
something before the try block, you receive the compiler message so and so
may not have been initialized. So, how do you want to handle this? What
should I then initialize the variables to? Should I create my own classes
that implement these interfaces, but do nothing, just so that the variable
can be initialized outside of the try block? That seems like a lot for
nothing.

And I do mean initialize, not just declare. If you don't even declare it,
you of course get a different message, because the variable does not even
exist in the finally block.
[ June 12, 2006: Message edited by: Martin Simons ]

[Martin Simons]

Originally posted by Tony Morris:
There is no need to assign a local reference to null. Embed your try blocks, or leak - as simple as that. Yes, using null as a dynamic placeholder to determine if a reference was ever assigned has the potential to cause a resource leak, albeit how common it is.

So you are saying a db lookup should look like this?

try {
  Class.forName("Driver");
  try {
    Connection conn = DriverManager.getConnection();
    try {
      Statement stmt = conn.createStatement("query");
      try {
        ResultSet rs = stmt.executeQuery();
        //work with rs
        rs.close();
      } catch (SQLException sqle) {
        // could not handle rs
      }
      stmt.close();
    } catch (SQLException sqle) {
      // could not create statement
    }
    conn.close();
  } catch (SQLException sqle) {
    // could not connect
  }
} catch (ClassNotFoundException cnfe) {
  // could not load driver
}

Wow talk about a lot of extra structure to avoid leaks that under most
conditions should not occur.

Talk also about the even larger possibility (under this construct) of not
closing statements/resultsets/connections due to a simple programming
error that will use up even more resources, quicker, than these rare
memory leaks.

Assigning a local variable to null does not hurt anything, once it goes out
of scope, it is out scope. Not closing the connection or whatever else,
is a much larger problem.
[ June 12, 2006: Message edited by: Martin Simons ]

[Mr. C Lamont Gilbert]

Originally posted by Martin Simons:

try {
  Class.forName("Driver");
  try {
    Connection conn = DriverManager.getConnection();
    try {
      Statement stmt = conn.createStatement("query");
      try {
        ResultSet rs = stmt.executeQuery();
        //work with rs
        rs.close();
      } catch (SQLException sqle) {
        // could not handle rs
      }
      stmt.close();
    } catch (SQLException sqle) {
      // could not create statement
    }
    conn.close();
  } catch (SQLException sqle) {
    // could not connect
  }
} catch (ClassNotFoundException cnfe) {
  // could not load driver
}

...
[ June 12, 2006: Message edited by: Martin Simons ]

No. It should look like this.

try {
  Class.forName("Driver");
  try {
    Connection conn = DriverManager.getConnection();
    try {
      Statement stmt = conn.createStatement("query");
      try {
        ResultSet rs = stmt.executeQuery();
        try {
          //work with rs
        }
        finally {
          rs.close();
        }
      }
      finally {
        stmt.close();
      }
    }
    finally {
      conn.close();
    }
  } catch (SQLException sqle) {
    // error of some sort
  }
} catch (ClassNotFoundException cnfe) {
  // could not load driver
}

[ June 12, 2006: Message edited by: Mr. C Lamont Gilbert ]