• Industrial Automation Engineer (unregistered)

    IF currentPost = Frist THEN HoldCommentForModeration(); END_IF;

  • Industrial Automation Engineer (unregistered)

    IF currentPost = Frist THEN HoldCommentForModeration(); END_IF;

  • Michael R (unregistered)

    If (postId == 3) { return "Thrid" } else { return "FILE_NOT_FOUND" }

  • Brian Boorman (google)

    I bet the compiler spits out the same machine code either way. If it's functionally correct and is just about style, is it really a WTF?

  • (nodebb)

    OK, condense it, then ask an average developer to set a breakpoint on on or the other condition... If you can get an 80% pass rate where there is no measurable difference in the time it takes the developer to do so, then great. Otherwsie the code (nearly) as written has value.

  • Tim Ward (unregistered)

    It might make sense for the method to exist, if, say, at different times or in different implementations of the virtual function, what's meant by "match" has varied, but the actual code is indeed ludicrous.

  • Sole Purpose Of Visit (unregistered) in reply to Brian Boorman

    You're missing the "hat he uck" of a WTF.

    Every time I see something like this in my own code base, another synapse snaps, another pair of neurons dies, and I lose another 0.000001% of the will to live.

  • Unregistered (unregistered)

    I would not write that myself, as a simple if statement is more readable. But it may be a case of over-applying a useful pattern, where complex flows are split up to be more readable.

    E.g.

    function registerUser(User user) {
    validateUser(user);
    saveUser();
}

    In this way the code becomes more readable. But it shouldn't be misused as in the example above

    (btw, I know there are better ways of validating a user, but this was just an example)

  • (nodebb)

    This looks like C# and as a C# developer I respectfully disagree that a method like this has any value, I strongly feel that it's actually a burden on any code base where such a pattern was applied.

    If I'm looking through code and see its use for the first time, perhaps like so:

    if (CheckIfIdMatch(currentProductId, foundProductId)
{
   // Do some logic
}

    I would most likely be wondering about the implementation of that method and be curious as to what the Id matching logic was, I would then navigate into the method only discover that there is no particular matching logic.

    If it was instead just:

    if (currentProductId == foundProductId)
{
   // Do some logic
}

    What was happening would have been immediately apparent and would not have made understanding the code needlessly harder. It even uses less characters!

    In terms of debug-ability, there are options such as watch, quickwatch or conditional break points depending on what you may need which would be far better than forever burdening fellow developers with harder to read code.

  • Bogolese (unregistered)

    Maybe it should be its own class? Maybe implemented as a singleton. And an abstract at that, with templated type subclasses. And a ternary. Always need a ternary. Because ternary. Yeah, make this "useful . . ."

  • dpm (unregistered)

    As someone who used to write assembler and C back when processor cycles were precious, the idea of pushing values onto the stack and calling a function is horrible. Just write the fscking code.

  • (nodebb)

    if (today==Wednesday) { delete(EasyReaderVersion); log(WARN,"You let us down, Remy!", wc_DISAPPOINTMENT); }

  • Scragar (unregistered)

    Seen similar logic before that had behaviour removed until it was just a straight comparison, but was originally intended to do more, so I'm mostly willing to let this kind of thing slide as long as the method is deprecated so no one tries to add new copies and/or removes existing references as a part of the warning clean up(which ideally happens whenever you make a change, but I've been in teams that only do this once a release).

  • (nodebb)

    I once worked with a company that required a change request for every line of code that was committed. The effect was that refactoring code that worked but could be better written never happened.

    The comment "// else return" tells me that the original author had just learned that they didn't need to use an "else clause" if the "if clause" had a return. Unnecessary comments is also indicative of a new programmer.

    I would not be surprised if the underlying WTF was change control rules that were too strict.

  • Kythyria (unregistered) in reply to Jonathan Lydall

    Conditional breakpoints are actually quite slow. If it gets hit enough times before the condition is true, it's possible that writing an extra if(condition) { Debugger.Break(); } and recompiling winds up being faster than using a conditional breakpoint.

  • Conradus (unregistered)

    I like how the "then" statement is contained in a block but the "else" statement isn't, just a little bit of inconsistency that's like a cherry on top of the sundae of awfulness. I have a sneaking suspicion that he originally wrote it without the block and didn't understand that the semicolon at the end of "return true;" was what was breaking his if statement, so he hacked at it until he stumbled upon putting into a block. Problem solved!

  • dusoft (unregistered) in reply to Conradus

    You seem to miss the point. There is no need for else block or else statement at all (and it's not used anyway). Just use the default return after condition and drop the else usage.

  • Barry Margolin (github) in reply to Conradus

    There's a popular philosophy of else-less programming. This generally involves replacing a chain of if/else if/.../else with sequential if blocks that each end with return, and the else block is just top-level code after the last one. This is just a simple example of it.

    Of course, these adherents would also use return <condition> to avoid the if entirely.

  • MaxiTB (unregistered)
    Comment held for moderation.
  • (nodebb) in reply to Kythyria

    Very true about them being slow and I have often done an if statement similar to what you've described, but such if statements and Debugger.Break statements should ideally never get committed and most certainly should never get merged into your main branch.

  • MaxiTB (unregistered)
    Comment held for moderation.
  • davethepirate (unregistered)

    ...and then one day a requirement comes along that says any "currentID" in the range of 4000-5000 should match a "foundID" of 6. Go change that everywhere.

  • I dunno LOL ¯\(°_o)/¯ (unregistered) in reply to Ross_Presser
    Comment held for moderation.
  • (nodebb) in reply to davethepirate

    ...and then one day a requirement comes along that says any "currentID" in the range of 4000-5000 should match a "foundID" of 6. Go change that everywhere.

    Chances are that the code base at that point uses a mixture of the function and == anyway.

  • Yikes (unregistered) in reply to Brian Boorman

    It's close, but not quite the same. You end up extending your code to include a function to compare integers. https://godbolt.org/z/8bosTxdY4

    What I presumed at first glance was that the comparison operation was being performed on an object/class of some sort, which would allow the comparison to change neatly if ever were the case, however this was sadly not true. The one plus is that no matter what you use for the ID variable names, there's an aspect of self-documenting code based on the comparison call (if it's actually used!).

  • (nodebb)

    I think you've missed an important point, by putting the ID comparisons into its own function it can be modified later to provide additional functionality. For example say you just want an approximate match on the product because anything close enough will do. So your new WTF checkIfIDMatch() would be:

    `fabs((float)currentProductId - (float)foundProductId) < epsilon)`

    and suddenly you've got fuzzy matching by changing only one line of code.

    (I have no idea what markup format is used here and can't find any documentation on it anywhere so apologies if there are spurious markup tags in the above, I used several in the hope that one of them is the form that's used here).

    Addendum 2022-08-26 01:45: Ah, looks like it's BBCode.

  • Vertel (unregistered)

    This honestly seems like being overly pedantic... is the code great? No. Is it bad? Also no. And with any decent compiler or interpreter this will be crunched down to a branchless version and inlined. So on one hand we have a not-great but human-readable version that will be compiled efficiently, and on the other hand we have technically-better-but-less-readable version... that will compile to the same code. Getting het up about this one is just being snooty, really.

  • WTFGuy (unregistered)
    Comment held for moderation.
  • WTFGuy (unregistered)
    Comment held for moderation.
  • Worf (unregistered)
    Comment held for moderation.
  • Jan (unregistered) in reply to davethepirate
    Comment held for moderation.
  • Elise (unregistered)
    Comment held for moderation.
  • Cathleen (unregistered)
    Comment held for moderation.

Leave a comment on “Expanded Conditionals”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article