Comment On InsertSql()

Data-driven applications need to generate SQL from time to time. Usually, we leverage things like stored procedures or ORM tools to keep our code sane, but from time to time, we might hard code our SQL statements. You sacrifice some flexibility for some transparency into what your code actually does to the database. [expand full text]
« PrevPage 1 | Page 2 | Page 3Next »

Re: InsertSql()

2013-01-09 08:03 • by Severity One
This makes perfect sense. If SQL would ever be translated into Swahili, you just have to change a couple of constants. It's called defensive programming.

Re: InsertSql()

2013-01-09 08:07 • by TheSHEEEP (unregistered)
This is pretty good.
I never thought of it myself, but the character used for a COMMA could actually change in the future...

Re: InsertSql()

2013-01-09 08:10 • by snoofle
I suppose if you really want to write truly defensive code, that you could write that procedure with strings and dynamically compile and execute it...

Re: InsertSql()

2013-01-09 08:10 • by Jules (unregistered)
What happens when you translate SQL into a language with different word order conventions, though? Adam's coworker needs to combine the flexibility of guarding against new rules of punctuation with this egregious example I had to deal with once:

string.format("{0} {1}, {2}, {3} /* snip */ ",
SELECT, COL1, COL2, COL3, ..., FROM, TABLE, WHERE, ...)

Re: InsertSql()

2013-01-09 08:16 • by Jorge (unregistered)
This is actually happening to me. The project manager insist that if it is a string, then it has to be a constant and the rest of the code should reference the constant. And there is not way to reason with this guy, he wants to rule applied globally and blindly.

Re: InsertSql()

2013-01-09 08:19 • by $$ERR:get_name_fail (unregistered)
That's what happens when you tell people "string literals are bad, [n]NEVER[/b] use them" - they start to create string literals like

COMMA = ','

or

OPEN_PARENTHESIS = '('

or my personal favorite:

LETTER_CAPITAL_A = 'A'.

Re: InsertSql()

2013-01-09 08:29 • by $$ERR:get_name_fail (unregistered)
398794 in reply to 398792
Jorge:
This is actually happening to me. The project manager insist that if it is a string, then it has to be a constant and the rest of the code should reference the constant. And there is not way to reason with this guy, he wants to rule applied globally and blindly.


return LETTER_I + SPACE + STR_PITY + SPACE + STR_YOU + DOT + DOT + DOT;

Re: InsertSql()

2013-01-09 08:40 • by commoveo (unregistered)
398795 in reply to 398792
Jorge:
This is actually happening to me. The project manager insist that if it is a string, then it has to be a constant and the rest of the code should reference the constant. And there is not way to reason with this guy, he wants to rule applied globally and blindly.


Does he not know that the .NET compiler does that automatically with string literals?

Re: InsertSql()

2013-01-09 08:44 • by Jack (unregistered)
sb.append(OPEN_PAREN);

should be

eval ( SBAPPEND OPEN_PAREN CONSTANT_OPEN_PAREN CLOSE_PAREN SEMICOLON )

except I can't figure out how to concatenate the constants, and I still haven't eliminated the parens.

Moar recursion mebbe?

XML?

Re: InsertSql()

2013-01-09 08:46 • by TheUnknownCoder (unregistered)
OMG! Ponies!

(trust me on this, there are hidden ponies here and there)

Re: InsertSql()

2013-01-09 08:48 • by Shock Puppet (unregistered)
398798 in reply to 398796
Jack:
sb.append(OPEN_PAREN);

should be

eval ( SBAPPEND OPEN_PAREN CONSTANT_OPEN_PAREN CLOSE_PAREN SEMICOLON )

except I can't figure out how to concatenate the constants, and I still haven't eliminated the parens.

Moar recursion mebbe?

XML?

Regex replace.

Captcha: jumentum, as in "Converting all of our strings to constants caused us to lose our jumentum".

Re: InsertSql()

2013-01-09 08:49 • by Smug Unix User (unregistered)
Nicely formatted SQL in code? Why not?
dim s as string = <sql>
Select
Col1,
Col2,
Col3
From
MyTable
<sql>.toString()

Re: InsertSql()

2013-01-09 09:02 • by Nagesh (unregistered)
If this code is called a lot, then standard Java™ practices state that you should use constants. When you use ",", it creates a whole new instance to a string which will need to be garbage collected (when the garbage collector gets around to it). The author is using StringBuilder, which has the advantage of creating only one extra string after all the constants are added. Whoever submitted this was a Java™ newb (or the real Nagesh).

Effective Java (Item 5), Joshua Bloch
Sun Microsystems, 2008

Re: InsertSql()

2013-01-09 09:09 • by dkf
Not enterprisey enough! It should make a call over the Enterprise Service Bus to a web service to do the construction of the queries to look up the constants for concatenating to make the query string. Like that it'll be fully future-proof.

(And if it's like real enterprise-grade software, it'll have the host IP address of the web service encoded as a number directly in the code. Maybe the MAC address too. It's not enterprisey if it isn't insanely fragile and sclerotic…)

Re: InsertSql()

2013-01-09 09:24 • by ObiWayneKenobi
Having things like the comma be constants is stupid and a WTF; having the column names be constants however is not, since you run the risk of spelling typos by hard-coding it. Other than the comma thing, this is a common "builder" pattern when one can't use an ORM.

Re: InsertSql()

2013-01-09 09:28 • by A developer (unregistered)
398803 in reply to 398792
Start changing the constants to make it really confusing for him and everyone else suchas this wonderful code:

NOT_TRUE = false
NOT_NOT_TRUE = true
NUMBER_ONE = 10
MY_NAME_IS_FUCKING_IDIOT = <managers name>

Re: InsertSql()

2013-01-09 09:29 • by Michael Perrenoud (unregistered)
You know, I've seen a lot of stuff like this, and I've probably even written stuff like this (YIKES!). But I find that most of the time the issue is that the programmer just doesn't understand the API's and what's possible with the framework.

I bet when the guy who originally wrote this looks back on it in ten years (since it's public now, haha), he's going to say, what was I thinking?!?

LOL - this is just awesome - I've seen so much stuff like this. Let me give you an example:

bool b = true;
if (b.ToString() == Convert.ToBoolean(true).ToString())
{
...
}

Re: InsertSql()

2013-01-09 09:30 • by What? (unregistered)
398805 in reply to 398800
Uh. NO.

Java will build a lookup table with strings so they all reference the same string (if the same string is indeed referenced multiple times) until the last reference goes out of scope; here GC will probably kick in a clean up the string.

So "," will not create a whole new instance each time.

[quote user="Nagesh"]If this code is called a lot, then standard Java™ practices state that you should use constants. When you use ",", it creates a whole new instance to a string which will need to be garbage collected (when the garbage collector gets around to it). The author is using StringBuilder, which has the advantage of creating only one extra string after all the constants are added. Whoever submitted this was a Java™ newb (or the real Nagesh).

Re: InsertSql()

2013-01-09 09:34 • by Nagesh (unregistered)
398806 in reply to 398805
[quote user="What?"]Uh. NO.

Java will build a lookup table with strings so they all reference the same string (if the same string is indeed referenced multiple times) until the last reference goes out of scope; here GC will probably kick in a clean up the string.

So "," will not create a whole new instance each time.

[quote user="Nagesh"]If this code is called a lot, then standard Java™ practices state that you should use constants. When you use ",", it creates a whole new instance to a string which will need to be garbage collected (when the garbage collector gets around to it). The author is using StringBuilder, which has the advantage of creating only one extra string after all the constants are added. Whoever submitted this was a Java™ newb (or the real Nagesh).

[/quote]

Do you care to back up that incorrect statement? I gave you my reference.

Re: InsertSql()

2013-01-09 09:43 • by $$ERR:get_name_fail (unregistered)
398807 in reply to 398795
commoveo:
Jorge:
This is actually happening to me. The project manager insist that if it is a string, then it has to be a constant and the rest of the code should reference the constant. And there is not way to reason with this guy, he wants to rule applied globally and blindly.


Does he not know that the .NET compiler does that automatically with string literals?


That's not the point. Defining string literals as constants has indeed various benefits, like making it a lot easier to change them later. I just stops being useful when you add string constants for things which are guaranteed to never change, like keywords in another language. Another case of following the rule but not the intention is to name string constants by their string content.

const ERROR_CODE_404 = "404"; // stupid
const ERROR_CODE_FILE_NOT_FOUND = "404"; // smart

Re: InsertSql()

2013-01-09 09:43 • by AC (unregistered)
My favorite:

StringUtils.EMPTY

Re: InsertSql()

2013-01-09 09:44 • by Charles F. (unregistered)
398809 in reply to 398806
Nagesh:
Do you care to back up that incorrect statement? I gave you my reference.

Java Language Specification section 3.10.5. String Literals:
"Moreover, a string literal always refers to the same instance of class String."

Here, read my white paper on String:
http://charles.forsythe.name/documents/10180/0/java.lang.String.pdf

Re: InsertSql()

2013-01-09 09:55 • by ubersoldat
Bah! At least it's not loaded from an XML file or from the database.

It's funny thought, that almost every project I've seen its code, ends up implementing its own ORM abstraction to do this kind of stuff.

Re: InsertSql()

2013-01-09 09:57 • by Anonymous (unregistered)
It might have been that the author was uncomfortable embedding code in strings (after all, an accidental keystroke that isn't noticed could break it, and nobody would notice until the code ran at run-time, or perhaps reviewed in source control). At least with symbol names the compiler can check at build-time instead of run-time for simple mistakes like that. I don't find it overly hard to read either. Unnecessary, but not the end of the world. I see it as a normal phase in a beginner's growth. EXP++;

Re: InsertSql()

2013-01-09 09:59 • by Sql() Like a Pig! (unregistered)
StringBuilder sbComment = new StringBuilder();
sbComment.append(FRIST); sbComment.append(EXCLAMATION);

//crap, took too long

sbComment.prepend(SPACE); sbComment.prepend(NOT);

return sbComment.toString()

Re: InsertSql()

2013-01-09 10:04 • by Dave H (unregistered)
398813 in reply to 398809
Charles F.:
Nagesh:
Do you care to back up that incorrect statement? I gave you my reference.

Java Language Specification section 3.10.5. String Literals:
"Moreover, a string literal always refers to the same instance of class String."

Here, read my white paper on String:
http://charles.forsythe.name/documents/10180/0/java.lang.String.pdf


Translation: I don't care if you ARE an aircraft carrier, I'm a lighthouse.

Re: InsertSql()

2013-01-09 10:05 • by joe.edwards
398814 in reply to 398809
Charles F.:
Nagesh:
Do you care to back up that incorrect statement? I gave you my reference.

Java Language Specification section 3.10.5. String Literals:
"Moreover, a string literal always refers to the same instance of class String."

Here, read my white paper on String:
http://charles.forsythe.name/documents/10180/0/java.lang.String.pdf

Successful troll is successful.

Re: InsertSql()

2013-01-09 10:13 • by Charles F. (unregistered)
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.

Re: InsertSql()

2013-01-09 10:29 • by Brian (unregistered)
The other WTF is the fact that they used a StringBuilder, but did not chain the append() calls.

Re: InsertSql()

2013-01-09 10:31 • by Overly Attentive Gizzard (unregistered)
398817 in reply to 398806
Nagesh:
Do you care to back up that incorrect statement? I gave you my reference.


Effective Java, 2nd Edition, Item 5:

The improved version is simply the following:

String s = "stringette";


This version uses a single String instance, rather than creating a new one each time it is executed. Furthermore, it is guaranteed that the object will be reused by any other code running in the same virtual machine that happens to contain the same string literal [JLS, 3.10.5].


From your own reference, only if you explicitly create a new string using the new operator does it create a new string. Good troll though, intentionally misreading the reference that points out you're wrong under the assumption that people would look for a /different/ reference instead of just read yours.

Re: InsertSql()

2013-01-09 10:34 • by Doozerboy (unregistered)
Why are you all assuming it's Java?

Could equally be c#.

Re: InsertSql()

2013-01-09 10:39 • by Nagesh (unregistered)
398819 in reply to 398817
Overly Attentive Gizzard:
Nagesh:
Do you care to back up that incorrect statement? I gave you my reference.


Effective Java, 2nd Edition, Item 5:

The improved version is simply the following:

String s = "stringette";


This version uses a single String instance, rather than creating a new one each time it is executed. Furthermore, it is guaranteed that the object will be reused by any other code running in the same virtual machine that happens to contain the same string literal [JLS, 3.10.5].


From your own reference, only if you explicitly create a new string using the new operator does it create a new string. Good troll though, intentionally misreading the reference that points out you're wrong under the assumption that people would look for a /different/ reference instead of just read yours.

My point is, even if it does reference the same literal, it is something that must be searched for at runtime (which with the binary search increases logarithmically with an increase in the number of string). In business applications, there are often millions of distinct String, and every time you look up that comma it costs you. How do you think it not be better to reference it directly?

Re: InsertSql()

2013-01-09 10:43 • by Doozerboy (unregistered)
398820 in reply to 398818
Doozerboy:
Why are you all assuming it's Java?

Could equally be c#.


Actually probably is java. The toString method in c# is ToString

Re: InsertSql()

2013-01-09 10:46 • by Bebert (unregistered)
398821 in reply to 398793
In some code I inherited, there is:

private static final int THREENUMBER = 3;
private static final int FOURNUMBER = 4;
private static final int FIVENUMBER = 5;
private static final int SIXNUMBER = 6;
private static final int SEVENNUMBER = 7;
private static final int EIGHTNUMBER = 8;
private static final int NINENUMBER = 9;
private static final int TENNUMBER = 10;
private static final int ELEVNUMBER = 11;
private static final int TWLVENUMBER = 12;
private static final int THRTEENNUMBER = 13;
private static final int FOURTEENNUMBER = 14;
private static final int FIFTEENNUMBER = 15;
private static final int SIXTEENNUMBER = 16;
private static final int SEVENTEENNUMBER = 17;

(yes, with the random spelling errors). This is used in gems like:

pstmt = conn.prepareStatement(updateQuery);
pstmt.setString(1, a);
pstmt.setInt(2, b);
pstmt.setTimestamp(THREENUMBER, c);
pstmt.setTimestamp(FOURNUMBER, d);

(with filed variable names to protect the innocent:)). Still wondering why 1 and 2 are still hardcoded. Where is the softcoding police when we need it?

Re: InsertSql()

2013-01-09 10:52 • by Charles F. (unregistered)
398824 in reply to 398814
joe.edwards:
Successful troll is successful.
Was that trolling? I thought it was a legitimate misunderstanding. There's a lot of misinformation about Strings in Java that circulates even among experienced developers.

That's why I wrote the document I referenced. I even cleared up some of my own misconceptions while I was writing it.

Re: InsertSql()

2013-01-09 10:56 • by Chase (unregistered)
398825 in reply to 398797
TheUnknownCoder:
OMG! Ponies!

(trust me on this, there are hidden ponies here and there)


Ponies? You mean unicorns and rainbows, right? I was surprised the first time I saw them too, but apparently there in every article Remy writes. Just view source and you'll see the cornify.

Re: InsertSql()

2013-01-09 10:58 • by Cyberzombie (unregistered)
398826 in reply to 398806
That would be true IF this was String concatenation. To take it a step further, String concatenation uses a lock to do its work (Strings are immutable.)

The author is using StringBuilder. Backing store is a char[], access is unsafe, and it's faster (no locks - 'tis why StringBuilders should be local to a method or thread-safe object.)

String constants are fine.

Re: InsertSql()

2013-01-09 10:58 • by Charles F. (unregistered)
398827 in reply to 398819
Nagesh:
My point is, even if it does reference the same literal, it is something that must be searched for at runtime (which with the binary search increases logarithmically with an increase in the number of string). In business applications, there are often millions of distinct String, and every time you look up that comma it costs you. How do you think it not be better to reference it directly?
OK, so you are trolling. Nobody is this clueless.

Re: InsertSql()

2013-01-09 10:59 • by Herr Otto Flick (unregistered)
398828 in reply to 398796
Jack:
sb.append(OPEN_PAREN);

should be

eval ( SBAPPEND OPEN_PAREN CONSTANT_OPEN_PAREN CLOSE_PAREN SEMICOLON )

except I can't figure out how to concatenate the constants, and I still haven't eliminated the parens.

Moar recursion mebbe?

XML?


This is why C++ allows string concatenation across quoted lines and macros:


#define OPEN_PAREN " ( "
#define INSERT " INSERT "
#define COMMA " , "

const std::string sql = INSERT
RSS_ITEM_TABLE_NME
OPEN_PAREN
NME_PUBDATE_COLUMN
COMMA
RECORDING_ID_COLUMN_NAME
COMMA
CHANNEL_ID_COLUMN_NAME
COMMA
NAME_OF_ENABLED_COLUMN
COMMA
...
CLOSE_PAREN;

Re: InsertSql()

2013-01-09 11:02 • by Singleton or Someting (unregistered)
398829 in reply to 398819
Please read http://www.xyzws.com/Javafaq/what-is-string-literal-pool/3

Re: InsertSql()

2013-01-09 11:10 • by Overly Attentive Gizzard (unregistered)
398830 in reply to 398819
Nagesh:
My point is, even if it does reference the same literal, it is something that must be searched for at runtime (which with the binary search increases logarithmically with an increase in the number of string). In business applications, there are often millions of distinct String, and every time you look up that comma it costs you. How do you think it not be better to reference it directly?


Sorry, I can only rate this as blue apples out of love, would not troll again. Moving the goalposts is not an interesting trolling technique anymore. U we're dueling sew welt b4, 2.

Re: InsertSql()

2013-01-09 11:17 • by Justsomedudette (unregistered)
398832 in reply to 398797
TheUnknownCoder:
OMG! Ponies!

(trust me on this, there are hidden ponies here and there)
All I see are rainbows and unicorns ;)

Captcha: Hey Persto! They appeared like magic.

Re: InsertSql()

2013-01-09 11:17 • by Raptor (unregistered)
398833 in reply to 398824
Charles F.:
joe.edwards:
Successful troll is successful.
Was that trolling? I thought it was a legitimate misunderstanding. There's a lot of misinformation about Strings in Java that circulates even among experienced developers.

That's why I wrote the document I referenced. I even cleared up some of my own misconceptions while I was writing it.

I think the user name "Nagesh" is a TDWTF meme, like Frist, Irish Girl, the embedded systems excuse, etc.

But for the record, I found your paper interesting. :) I even picked up a few things (like the + operator creating a new StringBuilder behind the scenes) though I already knew a lot of it beforehand.

Re: InsertSql()

2013-01-09 11:36 • by Paul Neumann (unregistered)
398835 in reply to 398792
Jorge:
This is actually happening to me. The project manager insist that if it is a string, then it has to be a constant and the rest of the code should reference the constant. And there is not way to reason with this guy, he wants to rule applied globally and blindly.
So, something like this?
Public Function GetItemInsertSql() As String

Const QUERY = "INSERT INTO RSS_ITEM_TABLE_NME " +
"(NME_PUBDATE_COLUMN," +
" RECORDING_ID_COLUMN_NAME," +
" CHANNEL_ID_COLUMN_NAME," +
" NAME_OF_ENABLED_COLUMN," +
" TITLE_COLUMN," +
" SUBTITLE_COLUMN," +
" DESCRPITION_COLUMN_NAME," +
" AUTHOR_NAME_COLUMN," +
" KEYWORDS_COLUMN" +
") VALUES " +
"(?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"
GetItemInsertSql = QUERY
End Function 'GetItemInsertSql
Of course that's ridiculous. Just expose the Const directly.

[ For constant strings only referenced once (as above in scope to the snippet), see Manifest Constant ]

Re: InsertSql()

2013-01-09 11:53 • by Zylon
398836 in reply to 398793
$$ERR:get_name_fail:
OPEN_PARENTHESIS = '('

And even worse, misspelled variable names.

Re: InsertSql()

2013-01-09 12:02 • by Nagesh (unregistered)
398837 in reply to 398827
Charles F.:
Nagesh:
My point is, even if it does reference the same literal, it is something that must be searched for at runtime (which with the binary search increases logarithmically with an increase in the number of string). In business applications, there are often millions of distinct String, and every time you look up that comma it costs you. How do you think it not be better to reference it directly?
OK, so you are trolling. Nobody is this clueless.

Really? What magic/quantum method are you using to look up that thread out of the pool?

Re: InsertSql()

2013-01-09 12:06 • by Mike (unregistered)
398840 in reply to 398815
You win you've now convinced the world that .Net sucks and Java is the way to go.

I agree that string literals should be compile time really strange that they are treated like variables in a framework that explicitly treats strings as immutable.

Re: InsertSql()

2013-01-09 12:19 • by Charles F. (unregistered)
398841 in reply to 398837
Nagesh:
Really? What magic/quantum method are you using to look up that thread out of the pool?
By hooking Geordi's visor to the deflector array, we are able to alter the timeline so that we had the comma before we needed it.

Also: thread?

Also, too: if you really are clueless and not trolling, you wouldn't understand the explanation anyway.

And: if you really are clueless and not trolling, you came to this site because someone posted your code to it. Go ahead, admit it.

Re: InsertSql()

2013-01-09 12:29 • by Captcha:modo (unregistered)
No no no no no no no! You don't use constants like this!

Doing
#define TEN 10

#define TWENTYFOUR 24
#define STR_HELLOWORLD "Hello, world!"
does not make sense, as the name already tells you the value (and if you change it it gets even more confusing). You should do it like this:
#define ITERATIONS 10

#define HOURS_PER_DAY 24
#define STR_GREETING "Hello, world!"





So this code snippet should be

private static String getItemInsertSql() {
StringBuilder sb = new StringBuilder();
sb.append(INSERTION_KEYWORD); sb.append(RSS_ITEM_TABLE_NME);
sb.append(START_GROUPING); sb.append(NME_PUBDATE_COLUMN);
sb.append(SEPARATOR); sb.append(RECORDING_ID_COLUMN_NAME);
sb.append(SEPARATOR); sb.append(CHANNEL_ID_COLUMN_NAME);
sb.append(SEPARATOR); sb.append(NAME_OF_ENABLED_COLUMN);
sb.append(SEPARATOR); sb.append(TITLE_COLUMN);
sb.append(SEPARATOR); sb.append(SUBTITLE_COLUMN);
sb.append(SEPARATOR); sb.append(DESCRIPTION_COLUMN_NAME);
sb.append(SEPARATOR); sb.append(AUTHOR_NAME_COLUMN);
sb.append(SEPARATOR); sb.append(KEYWORDS_COLUMN);
sb.append(END_GROUPING); sb.append(VALUES_COME_NEXT_KEYWORD);
sb.append(START_GROUPING); sb.append(CURRENT_INSTANT);
sb.append(SEPARATOR); sb.append(PLACEHOLDER);
sb.append(SEPARATOR); sb.append(PLACEHOLDER);
sb.append(SEPARATOR); sb.append(PLACEHOLDER);
sb.append(SEPARATOR); sb.append(PLACEHOLDER);
sb.append(SEPARATOR); sb.append(PLACEHOLDER);
sb.append(SEPARATOR); sb.append(PLACEHOLDER);
sb.append(SEPARATOR); sb.append(PLACEHOLDER);
sb.append(SEPARATOR); sb.append(PLACEHOLDER);
sb.append(END_GROUPING);

return sb.toString();
}


Ah, much better.

Re: InsertSql()

2013-01-09 12:30 • by mag (unregistered)
398844 in reply to 398815
Charles F.:
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.


LOL@efficiency, when has that ever been a concern? Then why do people use Java? See if his stack ever unwinded from a concern of efficiency, he would replace each append parameter with a string literal, then to be more efficient, he would skip the append calls and concatenate with operator+... then to be more efficient or readable, he wouldn't use a string builder and use a regular built-in String, then to be more efficient, he would put the string's into longer and readable lines to improve readability for efficiency of the company and compile-time.

Obviously this guy was taught Java in college, where he learned all them nice Object Oriented practices, never was very good at it, and probably chose Computer Science as his major because of the "job market."

Hmm, you can figure out a lot about a person by their code.
« PrevPage 1 | Page 2 | Page 3Next »

Add Comment