• (cs)

    "and is, in fact, quite brilliant."

    Well, yeah - it's clearly intended to be adaptable to an embedded platform with no database.

  • Jon (unregistered)

    #define ZERO 0

  • (cs) in reply to x
    x:
    CommentSystem.getSystem().inform("frist"); /home/Comments/Piecemeal-SQL:1: 'frist' comment not 'frist'. CommentSystem.getSystem().inform("frist"); ^ 1 error
    This guy wins.
  • SR (unregistered) in reply to dkf
    dkf:
    Justin:
    Most importantly, it reveals typos at COMPILE TIME not runtime.
    Only the most trivial kind. It doesn't guard against truly stupid things like swapping the WHERE and the SELECT...

    If you need to guard against that level of WTFery you need some new developers.

  • Anonymous (unregistered) in reply to Bonslaw
    Bonslaw:
    Anonymous:
    TRWTF is everyone going on about bloody embedded file systems. Childhood aside, since when did endless repetition = humour?

    It all started on 2010-01-11, when a post absent of WTF was posted on this very website. The comments started coming in. They were about embedded platforms, and file systems. At first it was frustrating that the same ideas were being posted again and again. Then, it became confusing. As we sat, pressing F5, we could only stare on in horror as more than two hundred readers all contributed their two cents about embedded file systems. To remain sane, all we could do was laugh.

    So do you still post "EVE ONLINE!" to every Error'd?

  • Montoya (unregistered) in reply to Anon
    Anon:
    halcyon1234:
    (Note: Going against all established standards, wooden tables will not be used in the process. They simply do not have the longevity needed to withstand the epochs ahead. Perhaps if they were some form of petrified and rpeserved wood, we will reconsider....)

    Still too risky. I'd suggest they be etched into corrosion resistant gold plaques as a back up.

    If you really want to make sure they last forever, store everything in a RAID array.

  • Hassan (unregistered)

    Maybe this was done for performance reasons? If the method BuildQuery is executed very frequently - we could get some gain, making those strings contant and not creating them each time the method is called.

  • czech (unregistered) in reply to Been there

    Doesn't seems to me any more wtf than

    #define TRUE  1
#define FALSE 0

    in C. I doubt that the interpretation of integer values in boolean expressions in C is going to change any time soon.

    But that's just me, I could be wrong.

  • BradC (unregistered) in reply to Been there

    and it's shorter, since you get to leave off the spaces and quotes every time.

  • (cs) in reply to Been there
    Been there:
    We have lots of code like this in the codebase where I work. There are several advantages:
    1. Your IDE doesn't warn you about strings that need localization, since those strings are now constants.
    2. You get auto-completion and typo-avoidance.

    Granted, most of our devs don't write code this way. But it's not wrong.

    Better yet, some IDEs give you SQL autocomplete & more inside the string. Yes, I'm talking about IDEA's language injection. You don't need "clever tricks" to get around making mistakes. You need tools to help you make less mistakes.

    edit: guess they never heard of StringBuilder either

  • (cs)

    Umm, TRWTF is that 1) he did not use an abstraction layer and 2) nobody else seems to find that odd.

  • Go Figure (unregistered)

    I don't think the string constants are used many times, probably just complying with some overzealous build process, spewing warnings about the non-externalized strings in code. Hungarian notation is somewhat of a WTF these days (the "mstr" prefixes meaning "member of type string"). It is not an optimization, as someone suggested, the compiler is clever enough not to create new objects if static strings are used in the code. And you can't tell from the snipet, whether PreparedStatements and positional parameter were used or not.

    The code is pretty safe and simply avoiding unnecessary warnings by using constants. The purpose of the class seems to be avoiding common typos while writing SQL queries and avoiding some code repetition.

    The real WTF: people thinking this code is WTF

  • Dionne Warwick (unregistered) in reply to DES
    DES:
    Umm, TRWTF is that 1) he did not use an abstraction layer and 2) nobody else seems to find that odd.

    You must be psychic if you can tell that by the posted code.

  • (cs) in reply to Been there
    Been there:
    We have lots of code like this in the codebase where I work. There are several advantages:
    1. Your IDE doesn't warn you about strings that need localization, since those strings are now constants.
    2. You get auto-completion and typo-avoidance.

    Granted, most of our devs don't write code this way. But it's not wrong.

    Yes it is, use a Stored Procedure, then you can track the name of the sproc in a constant. Much, much, better solution and it also works if Thundarr is roaming around with his amazing Sun Sword.

  • gus (unregistered)

    Yep, I've seen code like this, and the opposite, all within a few lines. This goes way back a bit, to CDC assembly language, but there was a compiler that defined "BCDBIT" to be "6" (The number of bits in a pre-ASCII character) and used that definition in about 33,000 places. But in almost everyplace it was used, it would break the code if BCDBIT was anything other than 6, as it usually was like shifting characters off the end of a 60-bit word.

    So 33,000 uses of a useless constant, then every few lines, a few "magic numbers", like 72 for the number of columns per line to read in. Grrrr....

  • Steve the Cynic (unregistered) in reply to czech
    czech:
    Doesn't seems to me any more wtf than 
    #define TRUE  1
#define FALSE 0
    in C. I doubt that the interpretation of integer values in boolean expressions in C is going to change any time soon.

    But that's just me, I could be wrong.

    You are wrong. Using the symbolic names in boolean contexts, and 0 and 1 in numeric contexts helps the poor schmuck reading the code know that you know that it really is a relevant type of context, or perhaps that you don't know. It's like explicitly comparing pointers to NULL, which shows you that I know that they are pointers, instead of testing them as if they are boolean variables.

    90% of what we do as programmers is talking to other people, and yet we allow tongue-tied incoherent nerds like ourselves to do it. Perhaps that's why there is so much WTF code out there.

  • The Laughster (unregistered) in reply to Anonymous
    Anonymous:
    Classic soft-coding example, nothing particularly special but sufficiently cringe-worthy I suppose. TRWTF is everyone going on about bloody embedded file systems. Childhood aside, since when did endless repetition = humour?
    This constant laughing in my head… can't you hear it? The laughter? The never-ending laughter, calling me to repeat jokes and create memes! Can't you hear it?
  • (cs) in reply to Steve the Cynic
    Steve the Cynic:
    90% of what we do as programmers is talking to other people
    What.
  • Synchronos (unregistered) in reply to Anon
    Anon:
    I'm thinking something like this:

    [image]

    But as we know from the previous future experience, they would anyway end up mangled like "s'ect" or "d'te". Or Voyager VI had only copper plates, not golden.

  • (cs)

    Now wait one cotton picking moment! Where the heck are the TABLE, VIEW & PROCEDURE constants! ;-)

  • sockonafish (unregistered)

    Seems like a brilliant way to open yourself up to SQL injection attacks once someone forgets to validate whatever is going into or coming out of that function.

  • (cs) in reply to Dionne Warwick
    Dionne Warwick:
    DES:
    Umm, TRWTF is that 1) he did not use an abstraction layer and 2) nobody else seems to find that odd.
    You must be psychic if you can tell that by the posted code.

    Not at all. With a proper abstraction layer (JDBC does not qualify), there is no need to write SQL code at all; the abstraction layer translates method calls on a query object into whatever query language the underlying RDBMS uses, which may or may not be some sort of SQL dialect.

  • Missing the point (unregistered)

    Your all missing the point. The advantage is you don't have to type "quotes" all the time. Just look at the queries, they are very readable and fast to construct

  • (cs) in reply to Wolfgang F.
    Wolfgang F.:
    That is why stored procedures was invented :D
    Not necessarily. I can think of two scenarios where stored procedures won't be a better solution. First, if the application has a reasonably flexible search feature, it may be necessary to build a query string as creating a sufficiently flexible stored procedure may be impossible (without using dynamic SQL in the sp, which defeats the whole point). Second, if the database lookup procedure takes an array argument. In many databases, the only way to implement this is to dynamically build an IN clause.

    This pattern usually develops when a developer has to deal with the first scenario above. Even though code in the article solves very few real problems and is quite WTF-worthy in implementation, the concept is valid.

  • Jimmy (unregistered) in reply to Been there
    Been there:
    We have lots of code like this in the codebase where I work. There are several advantages:
    1. Your IDE doesn't warn you about strings that need localization, since those strings are now constants.
    2. You get auto-completion and typo-avoidance.

    Granted, most of our devs don't write code this way. But it's not wrong.

    If you're writing magic sacrifice code to appease your angry IDE with no concern for readability or even sanity, then, yes, it is very wrong and you are very wrong.

  • Brian White (unregistered) in reply to Wouter van Kleunen
    Wouter van Kleunen:

    The sql statements are now checked compile time for spelling errors, instead of runtime. I think this is quite clever.

    It is quite clever. That is why there are these nifty things called stored procedures which fail to compile if you have typos.

  • somedude (unregistered) in reply to Anonymous
    Anonymous:
    Classic soft-coding example, nothing particularly special but sufficiently cringe-worthy I suppose. TRWTF is everyone going on about bloody embedded file systems. Childhood aside, since when did endless repetition = humour?

    You sir have obviosuly not seen Kung Pow. http://www.imdb.com/title/tt0240468/

    I suspect that the person who wrote this abstraction layer is the "34% sparklier" guy in this article: http://www.joelonsoftware.com/items/2009/09/23.html

  • (cs)

    This looks like someone had a good idea (not to use "magic strings" in code) and took it too far.

  • (cs) in reply to Jimmy
    Jimmy:
    If you're writing magic sacrifice code to appease your angry IDE with no concern for readability or even sanity, then, yes, it is very wrong and you are very wrong.
    Yeah, except that this, as already noted, actually makes queries easier to read and write.
  • James (unregistered)

    Very basic stuff...the removed snippet would contain the building if each clause, the initialization of the clause strings just ensures there's whitespace between them. Nothing to see here...

    captcha "jumentum" When Israel really gets moving on something?

  • Notes (unregistered) in reply to blakeyrat
    blakeyrat:
    Been there:
    We have lots of code like this in the codebase where I work. There are several advantages:
    1. Your IDE doesn't warn you about strings that need localization, since those strings are now constants.
    2. You get auto-completion and typo-avoidance.

    Granted, most of our devs don't write code this way. But it's not wrong.

    Yes it is, use a Stored Procedure, then you can track the name of the sproc in a constant. Much, much, better solution and it also works if Thundarr is roaming around with his amazing Sun Sword.

    Stored Procedures are nice, but not always needed. The real WTF here is that no one can read the SQL commands, or copy them into an interpreter shell.

    A single, clear SQL string can be easily tested to see that it is correct. This is one of the very few cases where I like copy-paste programming. The SQL interpreter shell validates the command, and then the programmer pastes it into the code, possibly changing "?" for the sample parameters.

  • Gargamel (unregistered) in reply to Been there
    Been there:
    We have lots of code like this in the codebase where I work. There are several advantages:
    1. Your IDE doesn't warn you about strings that need localization, since those strings are now constants.
    2. You get auto-completion and typo-avoidance.

    Granted, most of our devs don't write code this way. But it's not wrong.

    It's just poor man's LINQ.

    Captcha: commoveo [Imp.] watch (SO.) climax

  • (cs)

    Maybe TDWTF should have a poll associated with each article so commentors can vote if it is a WTF or not.

  • Alex (unregistered)

    TRWTF is that SQL keywords should be in capitals

  • drachenstern (unregistered) in reply to Notes
    Notes:
    The real WTF here is that no one can read the SQL commands, or copy them into an interpreter shell.
    Actually this code had a logger write out the executed SQL before passing it onto the server for actual execution, so it is entirely possible to grab the code and copy it into an interpreter shell. Especially with the judicious use of a few breakpoints.

    Captcha: tation (that's no moon, that's a battle 'tation -- aka: I got nothing)

  • c0rnh0l10 (unregistered) in reply to Hassan
    Hassan:
    Maybe this was done for performance reasons? If the method BuildQuery is executed very frequently - we could get some gain, making those strings contant and not creating them each time the method is called.
    Java. Strings. Immutable.
  • EngleBart (unregistered) in reply to Anon
    Anon:
    Anon:
    Still too risky. I'd suggest they be etched into corrosion resistant gold plaques as a back up.

    I'm thinking something like this:

    [image]

    I do not believe that you did not redact Pluto as a planet! Or change the vectors that pinpoint the precise location of earth. Do we really want the aliens to know our location? This engraving never should have made it past the IT security committee. I can see the argument before the committee now: Since we upgraded from the copper edition to the GOLD edition, we must be protected now. Maybe the aliens are responsible for the attack on Google...

  • JDBCanyone (unregistered)

    Err...a ton of comments and no mention of JDBC yet? This is Java code, shouldn't be puttng params in raw strings. Only one comment so far about injection, only a couple about escaping...

    Simply cannot believe this is being defended. It's apalling junk.

  • Luke (unregistered)

    Really these should have been stored in a database table. Hard-coding constants like that is bad practice. Ideally, the database table would be accessed via a web service for portability.

  • Dan (unregistered) in reply to Been there

    It's also arguably easier to read without the quotes around the string literals. Although I don't particular like this style, I generally only put things into constants which might change at some point, and SQL keywords clearly aren't one of those things. Also unless I really need to dynamically build SQL I put the entire SQL query in a single constant, which is much easier to read. I can't tell what the usage is in this example.

  • Anon (unregistered) in reply to Synchronos
    Synchronos:
    But as we know from the previous future experience, they would anyway end up mangled like "s'ect" or "d'te". Or Voyager VI had only copper plates, not golden.

    The plaque is from Pioneer, not Voyager. And it's actually gold anodized aluminum, but I think even Voyager's record was gold plated copper. The point about "s'ect" and "d'te" is well taken, however.

  • wyu (unregistered)

    This gives you compile-time warning for typos. That would be one positive aspect.

  • Mikey (unregistered) in reply to Been there

    Not wrong? Right, because who cares about avoiding SQL injection, or using stored procedures anyway, right?

  • (cs) in reply to Quastiophor
    Quastiophor:
    Loading the strings from XML would be ideal, but perhaps this was on an embedded platform where there was no file system.
    String comment = "<?xml version=\"1.0\" encoding=\"UTF-8\"?><!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Strict//EN\" \"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd\"><comment xmlns=\"http://www.w3.org/1999/xhtml\" xml:lang=\"en\" lang=\"en\"><message>We don't need no stinking filesystem.</message></comment>";
  • Homer (unregistered)

    Here are my results and now I want them in DESCENDING order. Oh Crap!

  • H Evans (unregistered)

    Why not just get the list of SQL keywords from the database ;)

  • doofus (unregistered) in reply to czech
    czech:
    Doesn't seems to me any more wtf than 
    #define TRUE  1
#define FALSE 0
    in C. I doubt that the interpretation of integer values in boolean expressions in C is going to change any time soon.

    But that's just me, I could be wrong.

    The WTF here is that the expression value zero has always been logical false. Every other expression value has always been logical true. Not just 1 (one), every non zero expression.

    IMHO we should always jump on the programmer who writes ...

    (expression == TRUE)

    ... and perhaps question the use of language :-)

    #define FALSE (0 != 0) #define FALSE ((! FALSE))

  • Aaron (unregistered) in reply to Been there

    I'd say the real WTF is using string concatenation to build the query (and probably not escaping it either), instead of using a parameterized query. In this case, the constants do help avoid type-os.

  • TimG (unregistered) in reply to Alex
    Alex:
    TRWTF is that SQL keywords should be in capitals
    As a result of the unfreezing process, I always read SQL as if it had trouble modulating THE SOUND OF ITS VOICE.
  • (cs) in reply to TimG
    TimG:
    Alex:
    TRWTF is that SQL keywords should be in capitals
    As a result of the unfreezing process,
    WELCOME - TO THE WORLD OF TOMORROW! [image]

Leave a comment on “Piecemeal SQL”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article