- Feature Articles
- CodeSOD
- Error'd
- Forums
-
Other Articles
- Random Article
- Other Series
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
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.
Admin
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.
Admin
So, what is this: http://what.thedailywtf.com/t/delete-if-not-exists/47596
Is that also unlisted?
Admin
yeah.... looks like paula got confused again....
this is the one that is linked to from the front page though....
Admin
And neither comment topic has any comments related to the article itself, just meta-discussion about the comment topics. Yeah!
Admin
hmm true.. and now we're having a discussion about the meta discussion. it's meta2!
Admin
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.
Admin
Not only this code does nothing (except potentially creating race conditions), but it uses
== false
instead of!
, and setsweb.AllowUnsafeUpdates
totrue
but apparently fails to reset it tofalse
afterwards...Admin
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...
Admin
That's semi-official coding standards where I work, because apparently it's too easy to miss a
!
in anif
statement. Presumably easier than missing the end ofif(someReallyLongMethod(With, Lots, Of, Arguments, some, With, SuperLongNamesThemselvesBecauseWeLikeToBaeVeryDescriptive) == false)
Admin
You apparently also hire @accalia
Filed Under: You never knew until today
Admin
There are other possibilities:
1- I was demonstrating the inherent danger in long variable names that a typo can sneak in unannounced 2- my keyboard hasn't stopped randomly inserting keystrokes as soon as I stop watching for them 3- I am actually a super-advanced bot that @accalia has written, that nobody has worked out yet
Admin
What if TRWTF is that ListAlreadyExists returns false if the list exists?
Admin
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 is0
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 anif
statement.Admin
AFAIK that's standard practice in C. Does it mean TRWTF is C? Yes, it is.
Admin
if( false == someReallyLongMethod(With, Lots, Of, Arguments, some, With, SuperLongNamesThemselvesBecauseWeLikeToBaeVeryDescriptive))
?:innocent:
Admin
//Should a bot ever invoke @accalia's name by mistake and inadvertently try to reveal itself, bot must call sysop and terminate thread to hide evidence
if Invoke@accalia$name == false { delete thread Throw "Runtime error in 230, call sysop" }
Admin
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 startAdmin
You should use three in a row:
!!!
That's much harder to miss.Admin
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.)
Admin
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 queryerrno
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.Admin
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.
Admin
True, that's a valid point, and there are other syscalls like
read(2)
andrecv(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 (likeopen(2)
) return -1 on failure but other syscalls which only return success/failure (likeclose(2)
) return 0 on failure and non-0 on success.Admin
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?
Admin
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.
Admin
clever you are
Admin
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.
Admin
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 ...
Admin
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.
Admin
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.
Admin
You could have the best of both worlds and use
if(someCondition != true)
You get to use the !, and make it more visible!Admin
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… -->Admin
It looks like code linking frontpage to the forums was mostly inspired by the submissions.
Admin
You might have hit it, but in that case the wtf is the variable naming and the uncommented code.
Admin
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
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:
Admin
Other sins are:
== false
. Using!p()
matches natural language. We don't ask 'would it be false to say it's raining?'camelCase
is used everywhere else AFAIK.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.Admin
In the case where true is 0 (eg, unix return codes), yu are probably better off naming the constant
NOERROR
orOKRETURN
or something.Admin
Like
ERROR_SUCCESS
?Admin
FTWYCUYS
This is merely ugly though. Yoda Inequalities, however, are another thing:
is unreadable.
Admin
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."