views:

379

answers:

3

Can the following code cause a memory leak? Would using a StringBuffer actually improve memory usage?

A little background: A coworker has been pushing his theories on memory leaks, and this is code he has identified as being problem code (without doing any sort of profiling), which he claims can cause a memory leak. I disagree with this, so I thought I'd put it to some other developers to get a third party opinion.

 List partCollection = new ArrayList()

 String partKeyID = null;
 String sPartNbr = null;
 String partDescription = null; 

 while(rsPartRes.next())
 {
        partKeyID = rsPartRes.getString("PART_KEY_ID"); 
        sPartNbr = rsPartRes.getString("PART_NBR"); 
        partDescription = rsPartRes.getString("PART_DESC");

        SomeValueObject someValueObject = new SomeValueObject();
        someValueObject.setPartKeyID(partKeyID);
        someValueObject.setSPartNbr(sPartNbr);
        someValueObject.setPartDescription(partDescription);

        partCollection.add(someValueObject);
 }

Assume that rsPartRes is a ResultSet in this code which could contain 100+ records. Basically, his concern is that because we are looping through this result set and not using a StringBuffer (which, in this case, I'm not even sure HOW you would use one), that it could be causing memory leaks. Is there ANY case that anyone sees here where this could possibly cause memory leaks or performance issues...?

+3  A: 

No, that won't cause a memory leak. However, it would be cleaner to declare the variables inside the loop:

 List partCollection = new ArrayList();

 while(rsPartRes.next())
 {
     String partKeyID = rsPartRes.getString("PART_KEY_ID"); 
     String sPartNbr = rsPartRes.getString("PART_NBR"); 
     String partDescription = rsPartRes.getString("PART_DESC");

     SomeValueObject someValueObject = new SomeValueObject();
     someValueObject.setPartKeyID(partKeyID);
     someValueObject.setSPartNbr(sPartNbr);
     someValueObject.setPartDescription(partDescription);

     partCollection.add(someValueObject);
 }

It's not even obvious why you need those variables at all:

 while(rsPartRes.next())
 {
     SomeValueObject someValueObject = new SomeValueObject();
     someValueObject.setPartKeyID(rsPartRes.getString("PART_KEY_ID"));
     someValueObject.setSPartNbr(rsPartRes.getString("PART_NBR"));
     someValueObject.setPartDescription(rsPartRes.getString("PART_DESC"));

     partCollection.add(someValueObject);
 }

(It would also be nicer to use generics, but that's a different matter...)

How was your colleague planning to use StringBuffer? There's no string manipulation going on here...

Jon Skeet
Jon, agreed on your coding suggestions. I'm not totally sure what the StringBuffer suggestion was. But I think my point would be exactly as you stated ... "There's no string manipulation going on here..."
JasonStoltz
+6  A: 

As far as I can tell, there is no need to use a StringBuffer here.

The only reason to use a StringBuffer to increase performance would be when you are concatenating Strings over and over.

String result = "";
while (condition) {
    result += somethingElse;
}

or (StringBuilder is a better replacement for StringBuffer these days)

StringBuilder result = new StringBuilder();
while (condition) {
    result.append(somethingElse);
}

The second piece of code performs much better.

jjnguy
I imagine that somewhere else in their codebase there is some code that usese this List and does the StringBuffer / StringBuilder work, probably to a webpage table. Of course this should be done in a JSP...
JeeBee
I'm marking this as the accepted answer. I think the answer I looking for is the fact that unless there is no concatentation or serious String manipulation going on, then there would be no need for StringBuffer.
JasonStoltz
Exactly !
jjnguy
A: 

What does he think is leaking?

Not sure how StringBuilder helps here since you don't appear to be building up (concatenating) any strings. Unless there is something going on inside SomeValueObject()

n8wrl