Win a copy of Kotlin in Action this week in the Kotlin forum!
  • Post Reply Bookmark Topic Watch Topic
  • New Topic

Inefficient code  RSS feed

 
Saathvik Reddy
Ranch Hand
Posts: 228
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi,

I was told this code is inefficiently using stringbuffer. What is the correct way to use of string buffer?


Could any one point what is inefficient about this code?
 
Henry Wong
author
Sheriff
Posts: 23283
125
C++ Chrome Eclipse IDE Firefox Browser Java jQuery Linux VI Editor Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator


The string addition in the parameter will create another string buffer, append all the components, then convert that to a string so the you can be appended to your string buffer.

Since you have a string buffer already, you can bypass all of that -- by appending the components to you stringbuffer directly.



Henry
 
Ernest Friedman-Hill
author and iconoclast
Sheriff
Posts: 24217
38
Chrome Eclipse IDE Mac OS X
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Well, first of all, note that if the code didn't use StringBuffer at all, it would be more efficient! This accomplishes the same thing without creating the unnecessary StringBuffer:



But I think the line your colleague was talking about was this one:

sb.append(" xxx: [" + getXXX() + "]\n");

The "+" operator is compiled into code using a StringBuffer and some potentially unnecessary toString() calls. By using "+" along with the StringBuffer, you're making some (very slightly) inefficient code (very slightly) more inefficient. What your colleague would like to see instead of this line would be:

sb.append(" xxx: [");
sb.append(getXXX());
sb.append("]\n");

But it really hardly matters, unless you plan on calling this millions of times in a tight loop.

Personally I'd probably write something like

public String toString()
{
return (getXXX() != null) ?
" xxx: [" + getXXX() + "]\n"
:
" xxx: null\n"
}

or I'd say "Hey, there's no reason why null shouldn't have square brackets, too, and write

public String toString()
{
return " xxx: [" + getXXX() + "]\n";
}
 
Rob Spoor
Sheriff
Posts: 21090
85
Chrome Eclipse IDE Java Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Another recommendation is using a StringBuilder instead of a StringBuffer. Much like Vector, StringBuffer is synchronized. And much like Vector, that synchronization is hardly ever used. StringBuilder can do anything a StringBuffer can, without the synchronization.
 
Joanne Neal
Rancher
Posts: 3742
16
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I read an article the other day that said there is an optimisation in Java 5 and 6 compilers that converts any string concatenation code (i.e. str1 + str2 + str3) to use a StringBuilder to produce the result.
It also argued that it would be a false optimisation to explicitly use a StringBuilder, because later versions of java may introduce even better optimisations of string concatenations that would not be applied if your code was actually using a StringBuilder.

I can't find the link at the moment but I'll post if I do find it.
 
Joanne Neal
Rancher
Posts: 3742
16
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
And here's the
link
 
Saathvik Reddy
Ranch Hand
Posts: 228
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks a lot for the replies.
This was simple but i couldnt figure it out by myself.

Thanks,
 
Mladen Grabowsky
Greenhorn
Posts: 29
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Joanne Neal:
And here's the
link

Excellent link Joanne, thanks!
 
Ilja Preuss
author
Sheriff
Posts: 14112
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
And here is a streamlined version that is as fast, and - in my opinion - easier to read:

 
Ernest Friedman-Hill
author and iconoclast
Sheriff
Posts: 24217
38
Chrome Eclipse IDE Mac OS X
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Ilja Preuss:
[QB]And here is a streamlined version that is as fast, and - in my opinion - easier to read:

Hey Ilja,

Great minds think alike -- go back and look at my post in this thread
 
Rob Spoor
Sheriff
Posts: 21090
85
Chrome Eclipse IDE Java Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Ilja Preuss:
And here is a streamlined version that is as fast, and - in my opinion - easier to read:


Actually, that code could be very very inefficient, if getXXX() takes a long time to return a result and that result is not null. After all, you are calling the method twice! Calling it once, then storing the result in a local variable would be a lot better.
 
Adam Schaible
Ranch Hand
Posts: 101
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Why are people obsessed with the ternary operator? The JVM/Compiler will optimize most of that, and I'd say the majority of us write in a CORBA - is that half of a nano-second really worth the anomaly?
 
Mladen Grabowsky
Greenhorn
Posts: 29
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

or I'd say "Hey, there's no reason why null shouldn't have square brackets, too, and write

public String toString()
{
return " xxx: [" + getXXX() + "]\n";
}

I'd go with Ernest's solution, it's the fastest, but more important, it is also the simpliest.
[ October 15, 2007: Message edited by: Mladen Grabowsky ]
 
Ilja Preuss
author
Sheriff
Posts: 14112
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Rob Prime:

Actually, that code could be very very inefficient, if getXXX() takes a long time to return a result and that result is not null. After all, you are calling the method twice! Calling it once, then storing the result in a local variable would be a lot better.


You are right.

On the other hand, if getXXX() just returns the value of a field or something, storing it in a local variable first *might* in fact be slower.

In most cases, it likely won't make a noticeable difference, of course.
 
Ilja Preuss
author
Sheriff
Posts: 14112
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Adam Schaible:
Why are people obsessed with the ternary operator? The JVM/Compiler will optimize most of that, and I'd say the majority of us write in a CORBA - is that half of a nano-second really worth the anomaly?


I'm not sure what you are getting at, but just for the record: I prefer the ternary operator in this case because of readability, not because of performance.
 
Ilja Preuss
author
Sheriff
Posts: 14112
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Ernest Friedman-Hill:

Hey Ilja,

Great minds think alike -- go back and look at my post in this thread


Ah, yes, I read that, agreed, and forgot about it...
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!