- Feature Articles
- CodeSOD
- Error'd
- Forums
-
Other Articles
- Random Article
- Other Series
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
After decade of experience with various db apps i only once encountered need of migration to different DB server. Well i wouldn't call it migration, because we had to rebuild whole app anyway...
Admin
Admin
Admin
@Charles F - actually, Java precompiles StringBuilder into String concatenation where possible. There is no performance impact here.
Admin
You're just presuming they're constants because they're in all caps. I'd guess that at least some of them are not, like "NOW"?
Admin
Using the concatenation operation (+), the entire string will be built at compile time, stored as a single constant in the class constant pool and loaded in a single bytecode (ldc).
It certainly seems possible for the compiler to optimize out the StringBuilder, but the Sun (Oracle) compiler certainly doesn't do that.
Admin
Lots of ignorance in the comments.
In general, if sb refers to an instance of a StringBuilder, then sb.append(x) has the same effect as sb.insert(sb.length(), x). Every string builder has a capacity. As long as the length of the character sequence contained in the string builder does not exceed the capacity, it is not necessary to allocate a new internal buffer. If the internal buffer overflows, it is automatically made larger.
http://docs.oracle.com/javase/1.5.0/docs/api/java/lang/StringBuilder.html
Admin
I'm not sure what all the comments about this being "inefficient" are about. To me, this looks like it's actually overly-efficient.
First off, the only way I can fathom this method existing in it's original form as it is in the article is if it's in a class that must be extended, which overwrites the (presumed) static variables being used to generate the field names and values. Otherwise, the insert statement never changes. Why would anyone ever need to generate the same insert statement every single time?
Assuming there are actually runtime variables that come into play here:
"INSERT INTO Table (foo,bar,bas) VALUES('" + foo + "','" + bar + "','" + baz + ")"
would be the suggested way you write it. String concatenation like this is in fact less efficient in Java than using a single StringBuilder and calling append.
I'm not going to touch the usage of the constants.
I wouldn't do it this way for sure, I'm just not sure how you can say it's inefficient.
Admin
Trick is always use parameter base approach to make insert statement or rely on ORM to do that for you.
Admin
People who follow "enterprise-y" rules so blindly would likely not bend common Java coding conventions, so it's probably a safe assumption.
If the values were not constants, the compiler would simply write the StringBuilder/append code for us, and the (+) operator would be a wash.
Using + for String concatenation is only "bad" if you split your concatenation up. For example:
Admin
http://docs.oracle.com/javase/tutorial/jdbc/basics/prepared.html
Dammit.
Admin
Yeah I'm an Idiot, I was confusing StringBuffer with StringBuilder.
Everyone is correct, let Java optimize it out:
http://stackoverflow.com/questions/7586266/is-chain-of-stringbuilder-append-more-efficient-than-string-concatenation
Admin
GOD DAMN I'm dumb :-/
Admin
Admin
Admin
They are probably running checkstyle, which has a "magic number check" that throws warnings for any hard-coded numeric values that are not in a constant. The default configuration makes exceptions for 0, 1, and 2.
Admin
Admin
QFT
Admin
What if you misspell the constant..?
Admin
After reading all the comments, I need to ask...
If you're making a (typically blocking) call to the db, does the efficiency of the mechanism you use to build the SQL string really matter all that much when your thread is going to block while waiting for the network/db-server/network/disks to do their thing and respond?
Yeah, I know it's the principal of the thing, but in practice...
Admin
Admin
I used to work here. I swear to god. The best part is, RSS_ITEMS had nothing to do with RSS.
Admin
Admin
For the record, JLS 15.28 explains what a constant expression is, and String values that are constant expressions are, according to 3.10.5 interned by the String.intern() method.
String itself is a final, immutable class, that's why interning works.
When a class is loaded, 12.5 states that a String instance is created for a literal iff the same instance is not already in the intern pool.
Admin
Admin
If you're using modern DB like Oracle, reader will never block anyone else.
Admin
You'd get a compile error..?
Admin
What exactly do you consider misspelled about that "variable" name? (The only misspelling I see is that "variable" should be "constant".)
Admin
Quick show of hands: how many here looked at the OP and immediately had more than eight QUESTIONs?
Admin
sb.append(BOBBY_TABLES);
Admin
Admin
I never liked Europe, anyways.
Admin
And for a degree of compile-time checking. Mistyping the ERROR_CODE_FILE_NOT_FOUND will probably be picked up by the compiler - mistyping "404" probably won't.
Admin
Overly-efficient... so, it's more efficient than it needs to be? That sounds a lot like inefficient to me...
Admin
In this particular case, no. But if you're doing something like assembling a "value in (1, 2, 3, ...)" clause by iterating over some input list, then potentially yes. I found a problem a few years ago that was doing that using string concatenation - and the huge amount of unnecessary allocating/discarding of string objects was driving the garbage collector insane.
So in general, I'd say use simple concatenation in all circumstances - except when there's looping involved. It's more readable that way, and the Java compiler can take care of the optimisations. In theory Java could optimise loops as well, but it doesn't do so yet.
Admin
More than eight questions? No, just one... WTF?
Admin
private static String getCommentInsertSql() { StringBuilder sb = new StringBuilder(); sb.append(LETTER_W); sb.append(LETTER_T); sb.append(LETTER_F); sb.append(EXCLAMATION); sb.append(QUESTION); return sb.toString(); }
Admin
Does that count even if the literals are constant variables instead of actual literals?
Admin
Sadly not to dissimilar from how ASP.Net Server controls generate HTML...
Admin
"Variables Won't Constants Aren't" - Dominic Connor attributes this to an article in Creative Computing but I read it before Creative Computing ever existed. Anyone know who actually wrote it?
Aren't won't aren't won't aren't - Not Sinatra Not to do or not to be, that is not the question - Not Shakespeare Not not not not not not not not not not not not not not not not not - Not Batman damnum - um yes TDWTF, it looks like you captcha'ed it perfectly.Admin
"I'm surprised that no one has pointed out that the the StringBuilder and append calls are a complete waste of CPU and memory. In Java, concatenation of String literals occurs at compile time. Instead of being stupid and inefficient, this method could at least be stupid and efficient."
I'll point out that in C# the same thing happens with literals as Java, so he could have done that which would have been much more efficient.
StringBuilder is more efficient when concatenating strings iteratively i.e. not literally; concatenating inside a loop will force the CLR to copy to a new string instance, since strings are immutable in .NET
Admin
What, nobody has remarked on the blatant need for a map construct here?
Admin
This can be made much more flexible by using the following approarch.
Admin
OMG, years ago I once did exactly this.
But later I realized its insanity, stopped doing it, and never spoke of it again.
I'm sorry you inherited my old code.
Admin
As an experienced Java programmer (~15yrs), I find it funny when people talk about efficiency, when they mean "useless micro-optimization in a piece of WTF code".
Admin
TRWTF is why did a unicorn pop up while I was reading this?
Admin
It can make sense. Say the maximum number of address lines in your code is 7, and you want to change it to 8. There should be a constant defined for it that you just change, but that doesn't keep people from using a hard-coded 7. So you do a search for use of the number 7. And get lots of false positives where 7 is used for the number of days in a week. So even though the number of days in a week isn't going to change, having a constant for it may make life easier.
Admin
+1. Been in this business 25 years. Built databases in Image, Sybase, Oracle, MS SQL, and Access. Never seen one migrated to another.
Admin
I've seen the rest of the code, and if they run checkstyle, they have turned off all other checks :) But you may be right, this code could be a paste job from someone else's job (who runs checkstyle). OMG.
Admin
Yes, terrible, I know. Just bored as hell.