• frist (unregistered)

    If not frist

  • Anon (unregistered)

    If (comments == null){ comments = "frist" }

  • bvs23bkv33 (unregistered)

    real wtf is space between function name and parentheses, and no space between arguments in argument list

  • (nodebb)

    At least this time, setting the variable to NULL does not mess up the calls to the function.

  • guest (unregistered)

    Version 1: result = GetPumpconfiguration( ... ); if( result==null ) result= GetPumpConfiguration( ... ); if( result==null ) result= GetPumpConfiguration( ... ); etc.

    At this point, someone thought: "All these if-statements are repeated, so they belong inside the function, to reduce code size!

  • Andrew (unregistered)

    The API has poorlry spelled paraameters.

  • Chronomium (unregistered)

    This honestly isn't all that bad - it's just taking the "if results null, try again" logic and moving it into the function instead of around it. Assuming that just calling the function doesn't have an Olmec's worth of overhead, it's pretty harmless - just confusing enough to not really be worth whatever benefits it was trying to make.

    Or, in a word, "clever".

  • Kashim (unregistered)

    This has the stink of a very poorly done database. We have one of these where I work, "First you try to look up the customer by name, but they aren't always listed properly, so then you try by IP, but sometimes those change and no one notes it, so then you try..."

    This could be a lot worse. Yes, initially looking at it as we are now, it is gross, but if they're doing similar calls all over the place, then the definition could make perfect sense. If they often have to do these awful retries, that syntax becomes pretty clear, and it's not like it took an hour to find out why it was written that way. Should it probably be split out into "RetryGetPumpConfiguration" that would do the null check, and then call into the other function? Probably, but I'm really betting that TRWTF is the database it is looking into.

  • (nodebb) in reply to Andrew

    The other TRWTF: "Oh, no. If the result paraameter has a value… "

    And the real TRWTF: A very poorly (poorlry?) conceived attempt at "caching" result data?

  • Gumpy Gus (unregistered)

    I've done similar things. It's not so bad. But I just called the funtion many times and have it set a global, not shuffle result in and out a dozen times. Slightly better? Worse?

  • Loren Pechtel (google)

    This doesn't strike me as all that evil. What I think is really going on:

    The database has a lot of old, incomplete data in it. The query is tried in several different fashions to cope with this garbage. While it's not good you have to live with what you have, there's probably not the budget to actually fix it.

    The right solution here, which almost certainly didn't exist when this was written, was null coalescence:

    Result = GetPumpConfiguration(...) ?? GetPumpConfiguration(...) etc.

  • I Am A Robot (unregistered)

    Barrel scraping time?

  • DrPepper (unregistered)

    if (result = GetPumpConfiguration(...)) return result; if (result = GetPumpConfiguration(somethingElse)) return result; .... return null;

    works for me.

  • DrPepper (unregistered)

    Looks like my newlines got removed on the previous post; I blame markdown.

  • Object delete. (unregistered) in reply to bvs23bkv33

    ... space between function name and parentheses ... There are only two reasons to NOT use that format unenforced:

    • Unquestioning copy of what others do
    • Being brought up with vi. That didn't have a reasonable 'go to next word or token' function BTW it was the format Eric Allman used for sendmail, which is a WTF for other reasons. All natural languages and math use it too.
  • Kashim (unregistered) in reply to DrPepper

    Blech, IFs. This is clearly a job for a single gigantic ternary.

    result = result ? result : result = GetPumpConfiguration( [search terms a] ) ? result : result = GetPumpConfiguration( [search terms b] ) ? result : result = GetPumpConfiguration( [search terms c] )....

    Sure, you could split that onto multiple lines, but what fun is that?

  • someone (unregistered)

    Why is getConfiguration being declared/assigned anonymous style?

  • Tom (unregistered)

    love me some null-coalescing operator in C#: result = getResult(thisWay) ?? getResult(thatWay) ?? getResult(otherWay); Still ugly but at least easier to understand.

  • Chris (unregistered)

    It seems to me that the method being called should be handling the retry logic. Or, doing one call to the database with the possibly-null fields left out, then working out which results fit the supplied parameters.

  • Guest (unregistered)

    This reminds me of something I saw in a code base long ago... There was a deploy script that (among other things) pushed some configuration to a J2EE application server, and one particular line of the script was repeated with the comment "do this twice to make sure it works". Sure enough, every time we ran it, the first line would fail and the second would succeed. We never figured out why... but "do this twice to make sure it works" became a running joke for us.

  • Little Bobby Tables (unregistered)

    Hm. Just needs a comment explaining why it's done this way. No big deal. Just, as Kashim says, a badly-designed database.

    We inherited a colossal ball of mud from a customer who employed us to replace their pre-millennial COBOL with a SOTA Oracle DB supported by something a little more contemporary. The DB we migrated into Oracle was decades old and as a result its data was, to use the technical terminology, all over the place. If the data was in one form: fine. If not, then it might be in a different form. Try that. If not, then it might be in a different form again. Try that. Oh good. and so on.

    The best fun we had was that we also had to reproduce each and every bug in their system so as to prove that we had completely duplicated their system. Then, and only then, were we allowed to go and fix those deliberately-introduced bugs.

  • unregistered (unregistered)

    To me it looks kinda like a system for loading configurations in a priority order where more specific rules are allowed to override more general rules.

    A complete match has highest priority. then a partial match without mgmtid and service then without mgmtid and format. then without service and format, and finally the most general rule that is constant over mgmtid, service, and format.

  • fa (unregistered)

    I expect this makes more sense and is easier to understand than a fluent API, for someone who hasn't seen either before.

  • Barf4Eva (unregistered)

    Man, that's a pretty weak link to a database problem.. a lot of assumptions to get to that analysis. (not saying it isn't, just saying not enough info provided) ... queryResult=result... article title called "if not null" ... meh.

    Perhaps it was a data-access problem? Which was actually the original PEBKAC problem? Or maybe queryResult is from something OTHER than a db... possible. Maybe someone was just "querying" an in-memory collection, w/ linq.

    Who da hell knows. That's TRWTF... :)

  • TCCPhreak (unregistered)

    I think this is one of these cases where some knowledge on Design Patterns and the GOF-Book greatly changes your perspective. As I just recently refreshed my memory on the "Chain of Responsibility"-Pattern I'm not exactly sure whether this code is really that bad. It's inverted: Normally, each element checks "can I work with this?" and passes to the next bridge post if it can't. Here, every function call checks whether a previous bridge post handled the task and will only touch it if no one else already was successful. You don't have to implement the pointer/reference to the next handler (or don't even need classes, objects any polymorphic dispatch) but can use the function, instead. Of course, the parameter list is hell and badly smells of other problems but the general chaining idea is kind of beautiful once you know how to read it. It sure is nicer than the constructor-daisy-chain I used.

Leave a comment on “If Not Null…”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article