• ben_lubar (disco)

    This topic is now unlisted. It will no longer be displayed in any topic lists. The only way to access this topic is via direct link.

  • accalia (disco)

    This topic is now unlisted. It will no longer be displayed in any topic lists. The only way to access this topic is via direct link.

  • NedFodder (disco)
    Comment held for moderation.
  • accalia (disco) in reply to NedFodder

    yeah.... looks like paula got confused again....

    this is the one that is linked to from the front page though....

  • HardwareGeek (disco) in reply to accalia

    And neither comment topic has any comments related to the article itself, just meta-discussion about the comment topics. Yeah!

  • accalia (disco) in reply to HardwareGeek

    hmm true.. and now we're having a discussion about the meta discussion. it's meta2!

  • HardwareGeek (disco) in reply to accalia

    Yes. However, eventually discussion of meta2discussion becomes uninteresting, at least to me.

    So, did S. T. remove the pointless code during the migration, or did he/she/it preserve the WTF and the original coder's angst-ridden comment for future maintainers to ponder?

    Filed under: INB4: Obviously, he/she/it preserved it for posterity by submitting it to TDWTF.

  • VinDuv (disco)

    Not only this code does nothing (except potentially creating race conditions), but it uses == false instead of !, and sets web.AllowUnsafeUpdates to true but apparently fails to reset it to false afterwards...

  • Gaska (disco) in reply to VinDuv

    It's just a snippet from the middle of some function. You can't make those judgement without knowing the full source. Though I have a feeling you are right...

  • Jaloopa (disco) in reply to VinDuv
    VinDuv:
    it uses `== false` instead of `!`,

    That's semi-official coding standards where I work, because apparently it's too easy to miss a ! in an if statement. Presumably easier than missing the end of if(someReallyLongMethod(With, Lots, Of, Arguments, some, With, SuperLongNamesThemselvesBecauseWeLikeToBaeVeryDescriptive) == false)

  • Kuro (disco) in reply to Jaloopa
    Comment held for moderation.
  • Jaloopa (disco) in reply to Kuro
    Comment held for moderation.
  • chreng (disco)

    What if TRWTF is that ListAlreadyExists returns false if the list exists?

  • VinDuv (disco) in reply to chreng
    chreng:
    What if TRWTF is that ListAlreadyExists returns false if the list exists?

    Or maybe DeleteList creates the list...

    That wouldn’t completely surprise me, though; we have in our codebase a struct with a member validity which is 0 when the associated value is valid, and some other value if it’s not. I always do a double take when I see it used in an if statement.

  • Gaska (disco) in reply to VinDuv

    AFAIK that's standard practice in C. Does it mean TRWTF is C? Yes, it is.

  • PJH (disco) in reply to Jaloopa
    Jaloopa:
    That's semi-official coding standards where I work, because apparently it's too easy to miss a `!` in an `if` statement. Presumably easier than ***missing the end of*** `if(someReallyLongMethod(With, Lots, Of, Arguments, some, With, SuperLongNamesThemselvesBecauseWeLikeToBaeVeryDescriptive) == false)`

    if( false == someReallyLongMethod(With, Lots, Of, Arguments, some, With, SuperLongNamesThemselvesBecauseWeLikeToBaeVeryDescriptive))?

    :innocent:

  • redwizard (disco) in reply to Jaloopa
    Comment held for moderation.
  • Jaloopa (disco) in reply to PJH

    Sounds like suspiciously modern thinking. The main proponent of this also goes for declaring all variables at the start of the method (usually in the form SomeLongClassName _SomeLongClassName = new SomeLongClassName()), and only recently accepted that early returns can be acceptable if they're right at the start

  • Zacrath (disco) in reply to Jaloopa
    Jaloopa:
    That's semi-official coding standards where I work, because apparently it's too easy to miss a `!` in an `if` statement.

    You should use three in a row: !!! That's much harder to miss.

  • machtyn (disco)

    The only idiotic reason I can think that this would exist (and it's probably because I've done something like this in one of my programming classes, caught my idiotic logic, and redesigned it) is that the "DeleteList" method ensures that the list really doesn't exist by attempting to create it, then deletes and/or destroys it (and then, maybe, forces garbage collection.)

  • Protoman (disco) in reply to Gaska
    Gaska:
    AFAIK that's standard practice in C. Does it mean TRWTF is C? Yes, it is.

    Only for process exit statuses and for POSIX system call returns. Most sane libraries (as well as Win32) use the much more sensible failure=0, success≠0 convention, with the caveat that an error code has to be returned either through an additional out parameter or with another function like GetLastError(). Of course, POSIX syscalls still require to query errno to get an error code, they apparently couldn't be bothered to return the error directly on failures and just return -1 in most cases.

  • tarunik (disco) in reply to Protoman

    Most of the problem with the convention that you describe is that 0 is a valid FD in POSIXville (it's stdin), while it's not a valid handle on Windows.

  • Protoman (disco) in reply to tarunik

    True, that's a valid point, and there are other syscalls like read(2) and recv(2) which could validly return 0 in non-error cases. I'm not saying POSIX is wrong or poorly designed (it's not), just that it ended up with an unintuitive convention. It would be even more confusing if syscalls that returned FDs (like open(2)) return -1 on failure but other syscalls which only return success/failure (like close(2)) return 0 on failure and non-0 on success.

  • LorenPechtel (disco)

    While this isn't good coding I'm not at all sure it's a WTF-level issue.

    We are simply assuming the list being tested for and the list being deleted are the same thing. What if it's updating the object to reflect changes to something else?

  • AwesomeRick (disco) in reply to LorenPechtel

    That was my thought. What if the DeleteList() method is for removing references to the list from other places? To me, this looks like teardown code - check if an object exists in some master registry, if it doesn't then it references to it should be cleaned up so it can be GC'd.

  • aliceif (disco) in reply to PJH

    clever you are

  • PC_LoadLetter (disco) in reply to Jaloopa

    I realize it's not your choice, but comparing against literals makes me want to scream. Hiring competent people with decent vision would be a safer measure.

  • tdwtf (disco)

    As we do not have all of the code we can only guess. I'll read it as: if the list "Title" does not exist on some object we do not need it on this object either We do not know what object ListAlreadyExists() work on, we do not know if web is that object or not.

    There is nothing hinting they are the same object ...

  • Gal_Spunes (disco)

    Dare I conjure JS nightmares by stating that we don't know what "this" refers to in the snippet.

    What if the logic here is to remove (cached?) lists from a local object (this) if they no longer exist in the "web" object?

    Just a thought...

    Of course, if true, then the real WTF is that the hapless commenter was too dumb to figure that out.

  • Shoreline (disco)

    Looks like the code wasn't tested properly, otherwise it would either be functional or non-existent.

    Also, since the same pattern is repeated 4 times, it would probably be a good idea to forEach [ "Title", "Self service", "Links", "Self service links" ].

    I'm assuming this is javascript. It looks like javascript.

  • prueg (disco) in reply to Jaloopa

    You could have the best of both worlds and use if(someCondition != true) You get to use the !, and make it more visible!

  • dkf (disco)

    You know what would reduce the WTF factor of this code? If ListAlreadyExists returns truthy if the list does not exist; then the rest of the code would make sense (though it would be written a bit stupidly).<!-- It does move the WTF around though… -->

  • obeselymorbid (disco) in reply to NedFodder
    Comment held for moderation.
  • kupfernigk (disco) in reply to Gal_Spunes
    Gal_Spunes:
    What if the logic here is to remove (cached?) lists from a local object (this) if they no longer exist in the "web" object?

    You might have hit it, but in that case the wtf is the variable naming and the uncommented code.

  • Peter_Wolff (disco)

    On a first glance, this looks like a common way to teach children at school without confusing them with concepts like sensible usage of types, e. g. bool, where the standard is indeed

    if (cond == true) ...
    if (cond == false) ...
    

    which I never was forced to use (thank God);

    but it would make perfect sense (in an intrinsical way) in an environment that implements my #1 favorite TDWTF:

    #define true 0
    #define false 1
    
  • Paul_Murray (disco)

    Other sins are:

    • using == false. Using !p() matches natural language. We don't ask 'would it be false to say it's raining?'
    • Uppercase names for methods. camelCase is used everywhere else AFAIK.
    • Explicit use of this. The whole point of inheritance and encasuplation is that code operates in a context that supplies a namespace.

    Other reason for not using == false is that there are four possibilities: ==/!= true/false. Having to unravel this logic is the reason the coder made this careless mistake in the first place. Not having to explicitly compare things to constants is the whole point of having boolean types. If you just name your predicate properly, then the sense of its return value is plain just by reading it.

  • Paul_Murray (disco) in reply to Peter_Wolff

    In the case where true is 0 (eg, unix return codes), yu are probably better off naming the constant NOERROR or OKRETURN or something.

  • dcon (disco) in reply to Paul_Murray
    Paul_Murray:
    In the case where true is 0 (eg, unix return codes), yu are probably better off naming the constant NOERROR or OKRETURN or something.

    Like ERROR_SUCCESS?

  • Bulb (disco) in reply to PJH
    PJH:
    if( false == someReallyLongMethod(With, Lots, Of, Arguments, some, With, SuperLongNamesThemselvesBecauseWeLikeToBaeVeryDescriptive))?

    FTWYCUYS

    This is merely ugly though. Yoda Inequalities, however, are another thing:

    if(2 > SomethingThatMustNotBeGreaterThanTwo)
    

    is unreadable.

  • tlhonmey (unregistered)

    Basho said to his disciple, "If you have a staff, I will give it to you. If you have no staff, I will take it from you."

Leave a comment on “Delete if Not Exists”

Log In or post as a guest

Replying to comment #:

« Return to Article