• Severity One (cs)

    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.

  • 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...

  • snoofle (cs)

    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...

  • 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, ...)

  • 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.

  • $$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'.

  • $$ERR:get_name_fail (unregistered) in reply to Jorge
    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;

  • commoveo (unregistered) in reply to Jorge
    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?

  • 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?

  • TheUnknownCoder (unregistered)

    OMG! Ponies!

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

  • Shock Puppet (unregistered) in reply to Jack
    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".

  • Smug Unix User (unregistered)

    Nicely formatted SQL in code? Why not? dim s as string = <sql> Select Col1, Col2, Col3 From MyTable <sql>.toString()<p> </sql></sql>

  • 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

  • dkf (cs)

    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…)

  • ObiWayneKenobi (cs)

    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.

  • A developer (unregistered) in reply to Jorge

    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=""><p> </managers>

  • 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())
    {
        ...
    }
    
  • What? (unregistered) in reply to Nagesh

    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).

  • Nagesh (unregistered) in reply to What?

    [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.

  • $$ERR:get_name_fail (unregistered) in reply to commoveo
    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
    
  • AC (unregistered)

    My favorite:

    StringUtils.EMPTY

  • Charles F. (unregistered) in reply to Nagesh
    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

  • ubersoldat (cs)

    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.

  • 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++;

  • 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()

  • Dave H (unregistered) in reply to Charles F.
    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.

  • joe.edwards (cs) in reply to Charles F.
    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.

  • 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.

  • Brian (unregistered)

    The other WTF is the fact that they used a StringBuilder, but did not chain the append() calls.

  • Overly Attentive Gizzard (unregistered) in reply to Nagesh
    Nagesh:
    Do you care to back up that incorrect statement? I gave you my reference.
    Effective Java:
    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.

  • Doozerboy (unregistered)

    Why are you all assuming it's Java?

    Could equally be c#.

  • Nagesh (unregistered) in reply to Overly Attentive Gizzard
    Overly Attentive Gizzard:
    Nagesh:
    Do you care to back up that incorrect statement? I gave you my reference.
    Effective Java:
    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?

  • Doozerboy (unregistered) in reply to Doozerboy
    Doozerboy:
    Why are you all assuming it's Java?

    Could equally be c#.

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

  • Bebert (unregistered) in reply to $$ERR:get_name_fail

    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?

  • Charles F. (unregistered) in reply to joe.edwards
    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.

  • Chase (unregistered) in reply to TheUnknownCoder
    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.

  • Cyberzombie (unregistered) in reply to Nagesh

    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.

  • Charles F. (unregistered) in reply to Nagesh
    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.
  • Herr Otto Flick (unregistered) in reply to Jack
    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;
    
  • Singleton or Someting (unregistered) in reply to Nagesh

    Please read http://www.xyzws.com/Javafaq/what-is-string-literal-pool/3

  • Overly Attentive Gizzard (unregistered) in reply to Nagesh
    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.

  • Justsomedudette (unregistered) in reply to TheUnknownCoder
    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.

  • Raptor (unregistered) in reply to Charles F.
    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.

  • Paul Neumann (unregistered) in reply to Jorge
    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 ]

  • Zylon (cs) in reply to $$ERR:get_name_fail
    $$ERR:get_name_fail:
    OPEN_PARENTHESIS = '('
    And even worse, misspelled variable names.
  • Nagesh (unregistered) in reply to Charles F.
    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?
  • Mike (unregistered) in reply to Charles F.

    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.

  • Charles F. (unregistered) in reply to Nagesh
    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.

  • 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.

  • mag (unregistered) in reply to Charles F.
    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.

Leave a comment on “InsertSql()”

Log In or post as a guest

Replying to comment #:

« Return to Article