Best way to do this?

[harryajh]

I have to process a large number of database records, examine a column
value (can be 10 different values) and set a bean's property depending
on that fields values e.g (vastly cut down)

ResultSet rs = ps.executeQuery();

while (true)
{
ret = new myBean();

String propName = rs.getString(2);

if (propName.equalsIgnoreCase("licence_number"))
{
ret.setLicenceNo(rs.getString(3));
}
else if (propName.equalsIgnoreCase("return_requirement_id"))
{
ret.setReturnReqId(rs.getString(3));
}
}

Hopefully its relativily easy to follow what I'm trying to do, my big
problem though is using a "String" to hold the fields value, this
obviously is not a good idea and surely for performance reasons
neither would -

if (rs.getString(2).equalsIgnoreCase("licence_number"))

or wouldn't it make much difference getting the field value up to 10
times?

A bit lost as to what's the best/efficient way to do this, should I
use a StringBuffer every time? would it be garbage collected? - any
other ideas?

thanks

harry

 

[Alan Gutierrez]

I have to process a large number of database records, examine a column
value (can be 10 different values) and set a bean's property depending
on that fields values e.g (vastly cut down)

ResultSet rs = ps.executeQuery();

while (true)
{
ret = new myBean();

String propName = rs.getString(2);

if (propName.equalsIgnoreCase("licence_number"))
{
ret.setLicenceNo(rs.getString(3));
}
else if (propName.equalsIgnoreCase("return_requirement_id"))
{
ret.setReturnReqId(rs.getString(3));
}
}

Hopefully its relativily easy to follow what I'm trying to do, my big
problem though is using a "String" to hold the fields value, this
obviously is not a good idea and surely for performance reasons
neither would -

if (rs.getString(2).equalsIgnoreCase("licence_number"))

or wouldn't it make much difference getting the field value up to 10
times?

Probably not, but it looks right to me to put into a local variable
something that you are going to reference 10 times locally, and the
method chaining looks repetitious.

A bit lost as to what's the best/efficient way to do this, should I
use a StringBuffer every time? would it be garbage collected? - any
other ideas?

Now for another installment of micro-optimization theater.

JDBC is going to give you a String. That String is going to be created
by JDBC. You cannot tell JDBC to reuse a StringBuffer or change the JDBC
API. (Cue the poster who shows me how you can use JDBC do just that. The
point is that if an API is returning a String there not much you can do
about it, and you shouldn't worry about it.)

In this microcosm of your application, when you get the String from the
ResultSet you are going use that same string for all 10 comparisons.
There is no place to insert a StringBuffer or StringBuilder here to make
things faster. Neither of them implement equals, so you would have to
create a String from them to do the comparison, or far worse, write your
own equality function that compares StringBuilder.

Short lived objects are cheap to create and collect.

You are not building a String, so StringBuilder buys you nothing, or if
it buys does you something I'm certain it is not worth the cost in your
time.

The slow part of this code is going to be the round trip to the
database, which might be adjustable, but the driver JDBC will probably
do the right thing and chunk up the transfers.

That is unless your large number of database records are part of a query
that is poorly designed. I'd run the query from the RBMS console get
timings, add indexes, try different joins, normalize, denormalize, and
hint until it was spewing rows quickly.

Basically, by the time you're fussing around with StringBuilder, the
real work has already been done, and the time it takes to do the real
work is what will determine its speed.

IJeff Atwood illustrates the futility of micro-optimization using a
String example, so this should really speak to you.

http://www.codinghorror.com/blog/2009/01/the-sad-tragedy-of-micro-optimization-theater.html

 

[markspace]

else if (propName.equalsIgnoreCase("return_requirement_id"))
{
ret.setReturnReqId(rs.getString(3));

^^^

}

Hopefully its relativily easy to follow what I'm trying to do,

Uh, no. Should that last getString() use a 4 instead of a 3? You use 3
twice in two different places.

problem though is using a "String" to hold the fields value, this
obviously is not a good idea and surely for performance reasons
neither would -

if (rs.getString(2).equalsIgnoreCase("licence_number"))

Not at all sure what you are trying to say or do. Maybe you should
hoist the string comparison out of the loop?

ResultSet rs = ps.executeQuery();

String id;
int propNum;

String propName = rs.getString(2);

if (propName.equalsIgnoreCase("licence_number"))
{
id = rs.getString(3);
propNum = 0;
}
else if (propName.equalsIgnoreCase("return_requirement_id"))
{
id = rs.getString(3);
propNum = 1;
}

while (true)
{
ret = new myBean();
switch( propNum ) {
case 0: ret.setLicenseNo( id ); break;
case 1: ret.setReturnReqID( id ); break;
}
}

This avoid string comparisons inside a loop....

 

[Alan Gutierrez]

Uh, no. Should that last getString() use a 4 instead of a 3? You use 3
twice in two different places.



Not at all sure what you are trying to say or do. Maybe you should
hoist the string comparison out of the loop?

I read the OPs uncompilable code as being a loop over a result set, even
though it has that nonsense while(true) which really should read while
(rs.next()), but then maybe the OP really wants an infinite loop over a
single row of a result set. In which case, it doesn't matter how much
faster it runs.

 

[Tom Anderson]

I have to process a large number of database records, examine a column
value (can be 10 different values) and set a bean's property depending
on that fields values e.g (vastly cut down)

ResultSet rs = ps.executeQuery();
while (true)
{
ret = new myBean();
String propName = rs.getString(2);
if (propName.equalsIgnoreCase("licence_number"))
{
ret.setLicenceNo(rs.getString(3));
}
else if (propName.equalsIgnoreCase("return_requirement_id"))
{
ret.setReturnReqId(rs.getString(3));
}
}

Hopefully its relativily easy to follow what I'm trying to do, my big
problem though is using a "String" to hold the fields value, this
obviously is not a good idea and surely for performance reasons
neither would -

if (rs.getString(2).equalsIgnoreCase("licence_number"))

or wouldn't it make much difference getting the field value up to 10
times?

Compared to the cost of going to the database, no, not at all.

An alternative approach would be to change your query to include "and
prop_name = 'license_number'", then you know that everything that comes
back is a license number, and so don't have to check. Then do the same
again for the other beans. However, that would rather increase the number
of queries you make, which is not likely to aid performance!

You could also do an N-way self-outer-join, which would put the values
stored against each property name in different columns in the result set.
I think the SQL for that would be a bit fiendish, though.

tom

 

[Lew]

Class names should start with an upper-case letter.

It looks like you might need to revise your database query or schema -
the same column of the result should not mean different things.

But since it does ...

I read the OPs uncompilable code as being a loop over a result set, even
though it has that nonsense while(true) which really should read while
(rs.next()), but then maybe the OP really wants an infinite loop over a
single row of a result set. In which case, it doesn't matter how much
faster it runs.

One way would be to have an enum keyed to the different possible
values from column 2 with a service method to set the correct value
from column 3, something similar to this uncompiled (let alone tested)
idea:

public enum ColumnSetter
{
licence_number
{
@Override
public void setColumn3( MyBean ret, String value )
{
ret.setLicenceNo( value );
}
},
return_requirement_id
{
@Override
public void setColumn3( MyBean ret, String value )
{
ret.setReturnReqId( value );
}
};
abstract public void setColumn3( MyBean ret, String value );
}

Then the client code is something along the uncompiled, untested lines
of:

...
PreparedStatement ps = Connection.prepareStatement( ... );
try
{
// set up query parms
MyBean ret = new MyBean();
for( ResultSet rs = ps.executeQuery(); rs.next(); )
{
ColumnSetter cs = ColumnSetter.valueOf( rs.getString( 2 ));
cs.setColumn3( ret, rs.getString( 3 )); // risks NPE
}
return ret;
}
finally
{
ps.close();
}

You can create a replacement method in ColumnSetter for valueOf() that
is case-insensitive. I usually define an instance override
'toString()' and static 'fromString()' for enums.

(As a riff on "RAII" I call the above try...finally idiom variously
"RRIG" or "RRIF" or "RRID", respectively "Resource Release is
Guaranteed", "Resource Release in Finally" or "Resource Release in
Deallocation". Your votes welcomed.)

 

[Andreas Leitgeb]

if (propName.equalsIgnoreCase("licence_number"))
else if (propName.equalsIgnoreCase("return_requirement_id"))

One thing to avoid unnecessary waste might be to "toLowerCase()" once,
and then use plain .equals() for comparisons.

Next step would be to look up the toLowerCase()'d property name in
a HashMap, that has all 10 strings associated with a small integer,
which you then use in a switch(...){...} to get the appropriate
method called on the MyBean for each number.


The "OO" way, would be to have some interface like
interface Storer { void store(MyBean b,String s); }
and in the HashMap you associate anonymous Storer-implementations:
Map<String,Storer> m = new HashMap<String,Storer>();
m.put("licence_number",new Storer() {
public void store(MyBean b,String s) { b.setLicenceNo(s); } } );
m.put("return_requirement_id",new Storer() {
public void store(MyBean b,String s) { b.ret.setReturnReqId(s); } } );
// ... ditto for the further 8 mappings

MyBean b = new MyBean();

Then, inside the loop:
m.get(rs.getString(2).toLowerCase()).store(b,rs.getString(3));

Well, actually you may want to break this up into separate statements
storing intermediate results in local variables and do null-checks
where appropriate.

For best encapsulation, you could add a method to MyBean, that takes
the Property's name and new value, and does the HashMap-lookup and
invocation of the obtained Storer. The HashMap itself would also be
maintained by MyBean and created and populated in the static initializer,
or lazily on first use.

I do not claim, that any of these would actually make your whole program
faster, but I dislike those if-chains, anyway, so even if your program
doesn't end up faster, I think those alternatives would still be worth
considering.

PS: "myBean" was a bad class-name and "GetString" is not a method of
ResultSet, so I changed them.

 

[Jim Janney]

One thing to avoid unnecessary waste might be to "toLowerCase()" once,
and then use plain .equals() for comparisons.

It depends. If you look at the code for String.equalsIgnoreCase, it
doesn't have that much overhead compared to .equals. If there are 10
choices that are all equally likely, each iteration of the loop will
average 5 comparisons, so you're balancing the extra cost of 5
case-insensitive comparisons against the possibility of creating a new
object. Note that equals or equalsIgnoreCase often return false
immediately after comparing lengths.

If the choices are *not* all equally likely, you can cut down on the
average number of comparisons by testing for the most common values
first.

 

[Jim Janney]

I have to process a large number of database records, examine a column
value (can be 10 different values) and set a bean's property depending
on that fields values e.g (vastly cut down)

ResultSet rs = ps.executeQuery();

while (true)
{
ret = new myBean();

String propName = rs.getString(2);

if (propName.equalsIgnoreCase("licence_number"))
{
ret.setLicenceNo(rs.getString(3));
}
else if (propName.equalsIgnoreCase("return_requirement_id"))
{
ret.setReturnReqId(rs.getString(3));
}
}

Hopefully its relativily easy to follow what I'm trying to do, my big
problem though is using a "String" to hold the fields value, this
obviously is not a good idea and surely for performance reasons
neither would -

if (rs.getString(2).equalsIgnoreCase("licence_number"))

or wouldn't it make much difference getting the field value up to 10
times?

A bit lost as to what's the best/efficient way to do this, should I
use a StringBuffer every time? would it be garbage collected? - any
other ideas?

thanks

Your first mistake is thinking that anything you do here will have a
measurable effect on performance. It won't: the run time will always
be dominated by the call to ps.executeQuery().

However, there's another mistake that I can't bring myself to ignore,
so: no, using a String to hold the field's value is not wrong, and
using a StringBuilder will not help you.

 

[Lew]

However, there's another mistake that I can't bring myself to ignore,
so: no, using a String to hold the field's value is not wrong, and
using a StringBuilder will not help you.

It's even worse than that. He asked about using a StringBuffer.

 

[harryajh]

It's even worse than that.  He asked about using a StringBuffer.

Many thanks for all replies to this, when I said it was "vastly cut
down" I didn't realise how much I had (there's a lot more that perhaps
I should have left in!), which seemed to cause a lot of confusion,
sorry for that chaps

My main point was the "String propName = rs.getString(2);" line,
thinking it was creating a object each time but actually thinking
about it the String has already been created by in the resultset and
therefore I think I'm right in saying this will not be a problem as
propName will just point to it?

cheers

 

[Lew]

Please don't quote sigs.

My main point was the "String propName = rs.getString(2);" line,
thinking it was creating a object each time but actually thinking

Why would that have been a problem?

about it the String has already been created by in the resultset and
therefore I think I'm right in saying this will not be a problem as
propName will just point to it?

'propName' does just point to it. There is a new 'String' allocated for each
string column of each row of each query.

You seem to be concerned with really microscopic issues. What is the real
risk that you seek to address and why?

 

[Alan Gutierrez]

Please don't quote sigs.

My main point was the "String propName = rs.getString(2);" line,
thinking it was creating a object each time but actually thinking

Why would that have been a problem?

about it the String has already been created by in the resultset and
therefore I think I'm right in saying this will not be a problem as
propName will just point to it?

'propName' does just point to it. There is a new 'String' allocated for
each string column of each row of each query.

You seem to be concerned with really microscopic issues. What is the
real risk that you seek to address and why?

I second. Did you have a look at that Jeff Atword article I linked to?
Did you draw anything from it?

Optimization should always be guided by profiling of some sort.

 

[Jim Janney]

Many thanks for all replies to this, when I said it was "vastly cut
down" I didn't realise how much I had (there's a lot more that perhaps
I should have left in!), which seemed to cause a lot of confusion,
sorry for that chaps

My main point was the "String propName = rs.getString(2);" line,
thinking it was creating a object each time but actually thinking
about it the String has already been created by in the resultset and
therefore I think I'm right in saying this will not be a problem as
propName will just point to it?

Yes, exactly.