• bvs23bkv33 (unregistered)

    null == frist

  • (nodebb)

    Not a fan of Yoda conditions, are we? (well, neither am I)

  • (nodebb)

    Personally, I'm left wondering what drove the cast in this line:

      stub = (ReportsWSStub)new ReportsWSStub(url);
    
  • Whatever (unregistered)

    Someone isn’t aware that putting the constant first protects against accidentally (and silently) performing an assignment inside of a conditional in languages that allow for it. I made great use of it back in my C days

  • (nodebb)

    If this bad code this isn't, then TA isn't me.

  • Whatever (unregistered)

    Someone isn’t aware that putting the constant first protects against accidentally (and silently) performing an assignment inside of a conditional in languages that allow for it. I made great use of it back in my C days

  • RLB (unregistered) in reply to Steve_The_Cynic

    The whole thing smacks of a beginning (or overly dogmatic) C++-programmer being forced to program in something which isn't even C.

  • Darkenon (unregistered) in reply to RLB

    I've seen code from a Company that migrated their code from C++ to Java but didn't hire anyone new. The code wasn't too dissimilar to this, lots of unnecessary typecasts. Also weirdly they kept setting variables to null at the end of functions due to what I assume is some way to try and manage memory.

    Overall, pretty weird code.

  • (nodebb)

    WRT the backwards null check, I recently learned that, in powershell, you must check null the backwards way. Actually, it's worse than that; the language doesn't enforce it, you'll just run into problems in time. Love powershell in general, has some serious quirks like this.

  • (nodebb) in reply to Steve_The_Cynic

    Well, you know,

    stub = new ReportsWSStub(url) as ReportsWSStub;

    could be null, and we do need a real stub object!

  • (nodebb)

    FristReferenceException

  • Guest (unregistered) in reply to bradwood

    Another reason for the "backwards" check is to avoid accidentally using the assignment operator instead of the equality operator.

    // example in languages where single equals sign assigns a value if (foo = 1) {...} //oops, you just assigned a value of 1 instead of testing for 1 if (1 = foo) {...} //you get a compiler error alerting you to bug in code

  • ooOOooGa (unregistered) in reply to Guest

    Some languages (Java maybe? C#?) require an actual boolean in conditional statements, so an accidental assignment will throw an error in any case.

    But in all languages that I have ever worked in, 'if (null = stub)' will blow up for an illegal assignment.

  • Erik (unregistered) in reply to BernieTheBernie

    ...I don't think that is the case. The constructor always either throws or returns an instance of the class its a constructor for.

  • Erik (unregistered) in reply to BernieTheBernie

    ...I don't think that is the case. The constructor always either throws or returns an instance of the class its a constructor for.

  • Somebody Somewhere (unregistered) in reply to Guest

    That "backwards check" pattern was certain useful back in the bad ol' days. But now, any compiler worth its salt will tell you "hey dummy, you're doing an assignment in what looks like it should be an equality check", and the backwards statements do nothing but break the mental flow of anyone trying to read the code.

  • (nodebb)

    There are still lots of compilers not worth their salt. Not a day goes by without dozens of Stack Overflow questions where the problem was an assignment that should be an equality test.

  • Fred McFredly (unregistered)

    Perhaps I am just "old school", but I am a big fan of so-called Yoda conditions, where the invariant is on the left hand side of the comparison operator and the variable is on the right. This catches an entire class of common errors (inadvertent assignment when) at compile time. That modern compilers will issue a warning is nice, but warnings are oft ignored. Much better that a logic error manifests as a compile error, so you have to fix it.

  • siciac (unregistered) in reply to Steve_The_Cynic

    You see all this defensive code, unncessary casts, redundant checks, etc. because the author doesn't understand what the language is doing. They may know a collection of facts, and they have an intuitive expectation, but they run the code and it breaks or does mysterious things, and rather than investigate why and form a coherent mental model, they guess at what might be wrong and hack at it. And this does eventually "work" well enough that they're able to push a change and close out a ticket.

  • Bubba (unregistered)

    Rumor has it that remyporter.com redirects here

  • Anon (unregistered)

    I like this line:

    log.error(" Exception", e);

    The log class doesn't have an overload with only the exception as a parameter? If not, create your own wrapper.

  • DrPepper (unregistered)

    The "redundant" check for null is not really redundant. This is a poor man's singleton implementation -- if there are multiple threads all trying to get access to the ReportsHelper object, then one of the threads will create it. All the others will just use the single object.

    Of course there are lots of race conditions present; but hey, this is "I didn't really want to write this" code.

  • (nodebb) in reply to Fred McFredly

    Modern compilers can turn selected or all warnings into errors. So where I'm working if (c = 1) will not compile.

    PS I hate yoda conditionals. almost as much as I hate how C/C++ parses if (a <= b <= c)

  • X (unregistered) in reply to DrPepper

    That would be true if stub was declared static (such that all instances refer to the same object) or if the check was not in the constructor, which only gets called once for each instance

  • (nodebb)

    I'm happy to see people defending the "backwards null check". I'll just add my own bit to that. Irrelevant for the shown case, but

    if the variable part of comparison is kinda long and/or complicated (object.something.blah.getThing()), then declaring the value compared against as the first thing lightens the mental load, in my experience.

    value-first reads something like "so the next part is supposed to have this value", while variable-first reads something like "so this expression that I'm still reading is supposed to have WHAT value?"

  • (nodebb) in reply to Anon

    That's just a lame logging statement message. Disappointingly common, but not a WTF of significance.

  • gnasher729 (unregistered) in reply to Guest

    Oh my god. For the last twenty years, I have used compilers that give a warning for "if (x = 1)", and turned on a compiler switch that turns all warnings into errors. if (1 == x) is for numpties who don't have proper tools or don't know how to use them.

  • Wizofaus (unregistered)

    I'd lose my 'enthisuasm' if I had to deal with code like this too.

  • Ulysses (unregistered) in reply to Fred McFredly

    Pfft, you're much more likely to make a logic error when you write yogurt inequalities. Doh, now I'm hungry.

  • Dlareg (unregistered) in reply to ooOOooGa

    Actually it would be a boolean c = 1. True assignment succeed c is now 1

  • RLB (unregistered) in reply to Guest

    Another reason for the "backwards" check is to avoid accidentally using the assignment operator instead of the equality operator.

    That's a great way of accidentally writing if (a = b) instead of if (a == b), and not being able to spot the mistake because you've trained yourself out of looking for it,

  • SyntaxError (unregistered) in reply to Whatever

    In java, you have to work harder to do an assignment where an expression is expected, so the null == someValue is really unnecessary and leads to cognition overhead (harder to read).

    if (someValue = null) {...} // Compile error

    while ((line = bufferedReader.readLine()) != null) {...} // No compile error, while valid, its nasty to read. And note the assignment is in parens, and then the value compared to null which evaluates to a boolean

  • SyntaxError (unregistered)

    Btw, this looks to be java. The give away being System.out.println(...);

  • JBanana (unregistered)

    Surely TRWTF is swallowing an exception in a constructor?

  • I'm not a robot (unregistered) in reply to Dlareg
    Actually it would be a boolean c = 1. True assignment succeed c is now 1
    No, in C-like languages, an assignment expression returns the value that was assigned (modulo assignment operator overloading shenanigans in languages that support that). In your example, "c = 1" has the value 1, and has the same type as c itself, presumably some sort of numeric type.

    This isn't the first time I've seen this silly meme of "an assignment returns true because the assignment succeeded" - anyone know where it comes from? Maybe there's some language where it's true, and people then think it's a universal rule?

Leave a comment on “The National Integration”

Log In or post as a guest

Replying to comment #:

« Return to Article