db-derby-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kristian Waagan <Kristian.Waa...@Sun.COM>
Subject Re: Re-using an existing db and archiving data
Date Wed, 19 Mar 2008 12:24:08 GMT
Mark Hiles wrote:
> I did both ways of testing to see if my resultset is null, which it isn't.
> 
> I can't upload the actual class I'm working on, I don't want the uni to accuse
> me of getting other people to do my code.. I'll type up something along the same
> lines, if someone could point me in the right direction of what I'm doing wrong..

Hello Mark,

I haven't followed this thread closely, but I just want to comment on a 
few things.

First of all, provide the stack trace (including line numbers) if you 
get an exception. This makes it a lot easier for others to help you.
The worst thing to do, is catching exceptions and ignoring them silently.

See some of my comments inlined below.

> 
> private ArrayList getSomeResults(String constraint) {
>   ArrayList<ArrayList> outerArrLst = new ArrayList<ArrayList>();
>   ArrayList<String> innerArrLst = new ArrayList<String>();
> 
>   try {
>     rs = stmt.executeQuery("select field1, field2 from tablename where field3 =
> '" + constraint + "'");

You should most likely use a PreparedStatement for this, along the lines of:

PreparedStatement ps = connection.prepareStatement("select field1, 
fields2 from tablename where field3 = ?");
ps.setString(1, constraint);
ResultSet rs = ps.executeQuery();
// process rs
rs.close();

If possible you should save the prepared statement reference so you 
don't have to prepare it all over again each time. Prepared statements 
are supposed to be reused.

> 
>     while(rs.next()) {
>             innerArrLst.add(rs.getString("field1"));
>             innerArrLst.add(rs.getString("field2"));
>             outerArrLst.add(innerArrLst);

To me this looks like you are adding the same list once for each row in 
the result set.
Your inner list will end up with field1 and field2 for all your rows, 
and the outer list will contain number-of-rows-in-rs elements, all 
referring to the same inner list.

>     }
>   } catch (Exception e) {e.printStackTrace();}

Be careful about catching Exception.
In this case I think you should consider what you can do with 
SQLException, a finally block and a throws clause for the method.
There are multiple options for handling exceptions in this method, the 
best one depends on your needs.

> 
>   rs.close();
>   rs = null;

No need for the rs = null here, it's a local variable and it's going out 
of  scope.


Just a few comments :)

regards,
-- 
Kristian

> 
>   return outerArrLst;
> }
> 


Mime
View raw message