• (nodebb)

    Based on the syntax, notably the :: separators everywhere, this is probably in C++, in which case, passing the QList by value isn't great (but maybe there's an actual valid reason for it). OK, if the list is short, and the copy isn't a fully-deep clone, it's not so bad, but ...

    Untodesu didn't supply the two line version, but based on the fact such a version exists, I also suspect that many of these branches weren't actually used. Or, at least, based on the actual business rules, could be combined.

    Most likely that those branches are theoretically possible, but in actual use, only that combination ever happens. Someone's going to get a surprise later, when something else changes and a now-missing branch becomes possible, or, worse, highly probable.

  • (nodebb)

    And on that day, a future employee will send the resultant shitstorm here...

  • (nodebb)

    A quick perusal of ISO/IEC 14882:2024(E) shows that, while the semicolon serves as "a statement terminator", the double-semicolon is defined as a "T-1000 terminator" --- used when you want to be absolutely certain that the statement is finished. Good to know.

  • (nodebb)

    Interesting: Member function that does not reference any member variables so should be static. The input parameter "type" is never used. Probably it was replaced with the hard-coded Stylus and they didn't want to change the function signature because they would have to change all the calling code or didn't have authority/ability to.

    I'm guessing there were unit tests for this function that encompassed all of the edge cases and needed to pass. Then someone poked around, made sure it wasn't possible to call this function with the edge cases, and decided to do an unnecessary rewrite that, as pointed out already, will probably break something in the future.

  • (nodebb) in reply to davethepirate

    or didn't have authority/ability to

    Thank you for bringing up memories of such unpleasant workplaces.

  • (nodebb)

    --defined as a "T-1000 terminator"-- So Arnold movies are in the code standards now?

  • Isn't it obvious? (unregistered)

    It's so easy to drop this to two lines, so long as you move most of the logic somewhere else in the program! I do see a redundancy though, namely checking for if(probeDesign.length() <= 1) when you're already sure that pos != 0 and pos != -1 and pos == probeDesign.length() - 1.

  • (nodebb) in reply to dpm

    Before C++ devs put their faith in the absoluteness of ;;, they should remember the T-1000 still lost.

  • (nodebb)

    well, the if pos is 0 and lenght is 1 can be carefully coalesced into if pos == length -1 block. but unless they got rid of the stylus and the pos = -1 thing i'm not sure how they'd get it down to 2 lines

  • (nodebb)

    How does that return statement work?

    QStringList TableViewAssembly::parametersFilter(ProbePart::Type type, int pos, QList<ProbePart> probeDesign) {
    QString to, from;
    // snip
    return { to, from };
}

    Somehow the compiler has to coerce that curly brace group into producing a QStringList. Shouldn't it be something like:

        return QStringList { to, from };
  • (nodebb)

    I hope that "rewriting it with a two-liner" doesn't mean simply re-implementing all of that logic as two deeply nested ternaries!

  • (nodebb) in reply to Gearhead

    Somehow the compiler has to coerce that curly brace group into producing a QStringList.

    Since C++11, the type is automatically figured out by the compiler and you don't have to write it explicitly. https://godbolt.org/z/vbvarhj3T

  • Loren (unregistered)

    This is not a bunch of bounds checking!

    This is a bunch of special case handling of boundaries.

    The core purpose of this function is to return what I presume is a pair of coordinates from the end of the previous object to the start of the next. Two lines. Everything else is concerned with what to do if there isn't a previous and next object.

    And your analysis is wrong. There are four scenarios: -1/start of list/in the list/end of list. There are five possible return situations, I'd hate to see what doing that in two lines looks like.

    But there is something here that's probably WTF worthy: The first four assign to, then from. The last four assign from, then to. Good way to confuse the person reading the code!

Leave a comment on “What Condition is This”

Log In or post as a guest

Replying to comment #699459:

Log Out

« Return to Article