• bvs23bkv33 (unregistered)

    Interpolat(frist)ion

  • (nodebb)

    I like the {filetrOptions.Type}. Sounds {meat}y.

  • David Mårtensson (unregistered)

    The main problem with this syntax is not string interpolation but having embedded expressions like that, its equally bad if you used string concatenation instead and even more difficult to read :/

    My guess is that it's the result of refactoring old concatenated strings.

    I know, we have a few in parts of the older code base at work.

    String Interpolation actually improved readability for those monstrosities but a real redesign of the query and how it is structured would probably be the only sane solution.

    A problem with alternatives is that it could make the query even harder to read OR duplicate the query for all alternatives, which lends it self very well to diversion where some of the queries are patches but not all.

    I have seen that happen to.

  • (nodebb)

    Even PHP doesn't allow interpolations inside interpolations!

  • (nodebb)

    The nesting of badness in this code is not too different from nesting of interpolation in its strings.

  • Sally Flynn (unregistered)

    Sweet merciful Chaos, someone fix that code before it tears a hole in the fabric of space-time with its awfulness!

  • zybex (unregistered)

    It's not that bad, it's still very readable. Personally I would do:

    string typefilter = filterByType ? $"AND (DataType IN ({filetrOptions.Type}))" : string.Empty; string query = $@"SELECT someColumn1, someColumn2, someColumn3 FROM myTable WHERE (TimeStamp >= '{startTime}' AND TimeStamp < '{endTime}' {typefilter})"

  • Mr Bits (unregistered)

    You've probably heard of the IOCCC (International Obfuscated C Code Contest). Today's WTF looks like a pretty good starting point for an entry into the IOC#CC.

  • Petr (unregistered) in reply to David Mårtensson

    Oh no, that is brand new code. It is part of automation test, which can sort of excuse the string sql, but it is still difficult to understand what is being tested.

    Typos are mine, not part of the code. Bonus point: DataType IN can only ever contain exactly one value.

  • (nodebb)

    I just within the last week concatenated strings to form a SQL statement. That's not the WTF, though, in this case I had to work around the ORM to make the statement exactly so. The WTF is that I didn't escape single ticks and didn't even wrap some values in ticks thereby creating SQL injection/butcher fail. In short order, we received a message from Azure SQL that there was probably fail in our code. And I've been doing this for 20 years. Take that.

  • DrPepper (unregistered)

    If it's part of test code, then maybe it can be excused. Otherwise you're wide open for sql injection. In terms of sql injection, interpolated string vs string concatenation is exactly the same.

  • Brian (unregistered) in reply to David Mårtensson
    David Mårtensson:
    My guess is that it's the result of refactoring old concatenated strings.

    Even for new code, tools like Resharper generally favor this form, and will helpfully suggest to refactor other forms of string construction (and will helpfully perform the refactoring for you). So devs that use such tools, especially in a shop where "no Resharper warnings" is part of the coding standard, tend to train themselves to write this way in all cases, even when there might be a (subjectively) better way.

  • (nodebb)

    Why the outright hatred for ternaries?

  • Chronomium (unregistered) in reply to BitDreamer

    Ternaries are a very attractive trap. It's a cool idea to distill a 5-8 line if/else construct into a smaller part of a larger line, but overall code readability suffers if it's any more complex than a pluralization tweak.

  • Carsten (unregistered)

    a poster and 14 comments all missing the sql injection is a sad WTF

  • ooOOooGa (unregistered) in reply to Zemm

    Nah. To do this in PHP you would have to use nested sprintf statements.

    $query = sprintf("SELECT $someColumn1, $someColumn2, $someColumn3 FROM myTable WHERE (TimeStamp >= '$startTime' AND TimeStamp < '$endTime') %s", $filterByType ? sprintf("AND (DataType IN (%s))", $filterOptions->type) : '');

    There, isn't that better?

  • Doodpants (unregistered)

    Woah, I didn't know you could make a single string both interpolated and verbatim (using both $ and @). I learned something useful today!

  • sizer99 (google)

    This is starting to be perlish - which is an obvious bad code smell.

  • sizer99 (google) in reply to BitDreamer

    Ternaries are lovely for what they're short, sweet, and very obvious like like ( items == 1 ) ? "" : "s". The primary rule is not to combine it with other control structures including other ternaries.

    But as soon as a code monkey has a hammer, they hammer everything and then you have FailSafe==0?'No technical alarms':(FailSafe&1)!=0&&(FailSafe&2)!=0&&(FailSafe&4)!=0&&(FailSafe&8)!=0?'Detection zones staying in a given state; Bad visibility; Initialization; Bad configuration':((FailSafe&1)!=0&&(FailSafe&2)!=0&&(FailSafe&4)!=0?'Detection zones staying in a given state; Bad visibility; Initialization': ((FailSafe&1)!=0&&(FailSafe&2)!=0&&(FailSafe&8)!=0?'Detection zones staying in a given state; Bad visibility; Bad configuration':((FailSafe&1)!=0&&(FailSafe&4)!=0&& (FailSafe&8)!=0?'Detection zones staying in a given state; Initialization; Bad configuration':((FailSafe&2)!=0&&(FailSafe&4)!=0&&(FailSafe&8)!=0?'Bad visibility; Initialization; Bad configuration':((FailSafe&1)!=0&&(FailSafe&2)!=0?'Detection zones staying in a given state; Bad visibility':((FailSafe&1)!=0&&(FailSafe&4)!=0?'Detection zones staying in a given state; Initialization':((FailSafe&1)!=0&&(FailSafe&8)!=0?'Detection zones staying in a given state; Bad configuration':(FailSafe&2)!=0&&(FailSafe&4)!=0?'Bad visibility; Initialization':((FailSafe&2)!=0&&(FailSafe&8)!=0?'Bad visibility; Bad configuration':((FailSafe&4)!=0&&(FailSafe&8)!=0?'Initialization; Bad configuration':((FailSafe&1)!=0?'Detection zones staying in a given state':((FailSafe&2)!=0?'Bad visibility':((FailSafe&4)!=0?'Initialization':((FailSafe&8)!=0?'Bad configuration':'Unknown')))))))))))))))

    See https://thedailywtf.com/articles/One-Bad-Ternary-Operator-Deserves-Another and https://thedailywtf.com/articles/Ternary-Operator-Nesting-Syndrome and https://thedailywtf.com/articles/Ternary-Over-a-New-Leaf

  • ZB (unregistered)

    TRWTF is that anybody calls string substitution "interpolation". In a logical universe, string interpolation would be putting in "A" and "Z" and getting back "M.N".

  • (nodebb)

    Ternary operators:

    They have a long history. While not specific to C, they could be used in such languages as (drumroll) Algol 60. It took a bit different form. You could say:

    a := if (b < c) then d else e;

    The C equivalent would be:

    a = b < c ? d : e;

    Yes, you can write bad code in almost any language, but some make it easier than others. You can also write "good" code in lots of languages, but that does take a bit of skill. How much skill is left as an exercise to the reader.

  • Gumpy_Gus (unregistered)

    Could be worse. The old VAX/VMS command language was very fancy, but the quoting rules were out of this world, so you would end up with a line containing all of the symbol interpolation symbols '' (two single quotes, used before a symbol), ' (single apostrophe, closes a '' symbol, " (double quotes, string delimiter, and & and &&, last-pass symbol delimiters. Not quite as bad as Perl or TECO, but getting close.

  • Vilx- (unregistered)

    With all due respect, if this is the query you need to produce, I think this actually is one of the sanest ways to do it. Sure, it takes some brainpower to decode it -- but every other option needs even more. It's easy to make fun of tangled code, but we tend to forget that often the tangle comes from the business requirements, not the programmer. If the business requirements are spaghettified, then even the best code it the world will be spaghettified.

  • HK-47 (unregistered) in reply to Vilx-

    You don’t construct queries by concatenating strings, you use parameter binding.

    Only a Sith meatbag would stitch strings together, Jedi use binding.

  • WTFGuy (unregistered)

    @HK-47: In general I agree that parameter binding is the way to go. But that works elegantly IFF your query's WHERE logic is constant and only the values to be compared change at run time.

    But if you have a business need, as this example apparently does, to query based on several optional parameters, then dynamic construction begins to make better sense. You just need to be darn sure you've sanitized the heck out of any supplied values. The alternative is that every one of your comparisons turns into something like "WHERE ... AND (@OptionalParameter1 IS NOT NULL AND <comparison involving @OptionalParameter1>) which idiom works fine until one of your business requirements is to look for NULL values in some cases. There are several other complications down that rabbit trail.

    The other option, mentioned by David Mårtensson above, is to create a separate query for each possible combination of comparison options and supply only the relevant values by parameters. But the combinatorics of that approach quickly get out of hand and the example apparently includes "several" of these optional comparisons. If we assume "several" = a total of only 5 then we have 2^5 = 32 almost identical queries to create, debug, and maintain. Good luck with that.

    As Vilx so wisely said: If the business requirements are spaghettified, then even the best code it the world will be spaghettified.

    As the submitter Petr later told us, this is test harness code, and gets a partial pass on that basis alone. At least it would if I was in charge of his shop.

  • Altaree (unregistered)

    Inception became a thing because it is a heist movie where the viewer actually has to pay close attention or they will miss something.

  • siciac (unregistered) in reply to ZB

    Damn it. Now I'm going to be irritated every time I see that.

Leave a comment on “Interpolat(interpolation)on”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article