- 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
What it someone replaces false with FileNotFound?
Admin
I would have used if (Frist === Frist)
Admin
TBH I've been to a number of companies but only two had code review, one of them a real code review, the other one a meeting called "code review"
Admin
Of course assuming Remy's completely reasonable theory is accurate then the intended production code would have looked something like this:
Which is still the ever-popular double negative logic. Plus the ever-popular compare bool to bool to over-determine truthishness.
Yes folks, in just 1 short line of code we have 3 stupids. As WTFs go this may not be very large, but it sure is dense.
Admin
That depends on how much the company abuses thruthiness. If configFlags.UseTabNavigation could also be zero as a valid alternative, then that is the only way to test for 'false'.
Of course that only shifts the WTF over to abusing truthiness, but still - put the fault where it should be.
Admin
I always write out "== false" if I check if something is false. Because that tiny little "!" can be overlooked so quickly and imo invites you to write messy code because it looks so tiny, nice, and clean at first, but can become really ugly once you introduce braces:
Of course one might argue that if you ever feel the need to write an if-statement like this your code should be refactored ASAP, but since we all know ASAP will never happen it sits there for years to come and that "!" between the braces WILL get overlooked by some poor fellow who gets thrown at the code.
Admin
I like the way it uses the Favascript identity operator, rather than the equality operator. This may actually be future-proofing a WTF (if there is any such concept), since you can now introduce the fabulous "True, False, FileNotFound" Boolean, and it will actually do the right thing!
(I was going to correct the mis-spelling up there, but for … reasons … I'll leave it as is. Mine's a chianti!)
Admin
Usually when I see this sort of thing it's because the developer was coding up the two different view modes before any thought had been given to the menu or config file (or whatever) that would implement switching between them.
It's a sloppy shortcut but the biggest mistake is forgetting to delete it when it became unnecessary. For Remy's theory to be true then testing in that place would be so sloppy that they'd managed to ship a product with a chunk of functionality not accessible ... I can't ever imagine that happening in the real world :D
Admin
I feel like the right approach is just to pull some of those inner expressions into locals. And on that note, I've actually seen a classmate do
someLocal =! someComplicateExpression
specifically to make sure the bang can't be overlooked.Admin
Do you think you could clarify this? The idiomatic approach in Javascript is just to use the implicit conversions - zero is false, NaN is false, any other number is true, etc.
Admin
If truthiness is being abused, configFlags.UseTabNavigation could be given the value of 0. At that point (configFlags.UseTabNavigation === false) would be false, but (configFlags.UseTabNavigation == false) would be true.
Another problem is with that part right there.
In this example where the code is checking for a particular value it could be made to work. Check explicitly for a particular enum value or a defined constant.
But checking for 'not false' becomes problematic. There are a lot of 'not false' values. So checking that (notFalse_a == notFalse_b) doesn't always work as expected. Not usually a problem within a single language. I have mostly heard about this in things like marshalling.
Admin
I still maintain that the real WTF is a language the has both the == and === operator.
Admin
@doubtingposter: The whole and entire point of a class like my purported configFlags is to sanitize all those input issues so downstream code doesn't have to be written defensively in umpteen different locations for every possible way the businessidjits could configure the config file. We've all seen logicals where the business expects "yes", "true", 1, -1, "True", Wahr,12 etc (but not "TRUE" or 14) to be acceptable inputs meaning logical truthiness. Good technique is to clamp that crap down in exactly one place, not to try to code defensively against it everywhere.
@unregistered I will certainly agree that ...
someThing == false
is safer thansomeThing == true
when you're dealing with multiple mixed languages / environments or lazy devs who can't be bothered to know how their chosen enviroment actually works. Across all languages and all times there's not a lot of agreement on which values mean "true", but there's pretty much universal agreement on which values mean "false".Still, using ==false everywhere seems to me to be applying a code smell everywhere to overcome the occasional bit of stinky cross-platform-ity that might slip in in unnoticed. Far better IMO to explictly deal with the cross-platform or cross-language interop issues at the immediate point of interface and comment that's why you're doing what you're doing where you're doing it.
Then all downstream code can be written using the natural idiom / OM of their language and be assured that values representing logical true and false are idiomatically unambiguously true or false with no
FILE_NOT_FOUND
s booby traps slipped in.Admin
I think we've all written code like this, when trying stuff out - and most of us have probably had it slip into production. I know there's been some if(false) { ... } blocks in our codebase in the past.
Admin
We started doing it 'properly' (every commit merged up to the main branch has to be put through a PR) a couple of years ago. It adds significant overhead, and if the team all knows what it's doing it isn't worth it for small things. But to be fair the reviews do turn up quite a few things, both 'oops I used < when I meant >' or 'yes I forgot to unit test that scenario' kind of things and also misunderstandings of design or component boundaries. So although I bristle at having my code reviewed, they are a good thing and imo worth the time.
Admin
This:
if (a && !(!(b+c)-d)) then ...
firstly, those are brackets, not braces.
Secondly anyone who uses logical operators to coerce arithmetic values to a truthy value deserves everything that happens to them
Addendum 2020-06-18 15:05: and also uses arithmetic operators in truthy values
Admin
I also like the absurdity of if( false == false ) being true.
Though I bet there must be a language where ( false == false ) is not true, like SQL and its NULLs. JavaScript is crazy enough I bet you could make this happen.
Addendum 2020-06-18 15:12: Actually, you could do this in python on a technicality - 'false' means nothing in python (the boolean is 'False') so you could make false an object and override the equality test.
Admin
don't get me wrong I'm all for code review, unfortunately most of the companies don't deem it necessary
Admin
I assumed what happened was the reverse. They originally had two different options, but "startTabNavigation" was too buggy, became too difficult to maintain, or became redundant, so they just chopped it out in that if statement.
Admin
"I think we've all written code like this,". No, never.
Admin
It's not just about the quality of the code being reviewed, though — there's often also benefit from exposing other members of the team to the code you're working on... partly the training aspect and reduced bus-factor, and also because it helps with communication among the team, especially when several people are working in the same area and can review each other's work.
Admin
It's to be expected in a loosely typed language.
Admin
With my compiler, if (false) { ... } will give a warning because the condition is always false, and warnings are errors, so it doesn't compile.
You can write if ((false)) { ... } with an extra pair of parentheses. The compiler now assumes that this is not a mistake that you made, but it is fully intentional that the following code cannot be executed, therefore no warning.
Admin
The whole double-negative logic is a bad habit left over from programmers who were ever exposed to COM with Visual Basic 6. This is where the definition of false is "0", and the definition of "true" is "-1". This is counter intuitive to many C / C++ programmers who will have defined FALSE as 0 and TRUE as 1. Therefore testing the negative case was often more reliable. Another similar idiom (which kinda makes sense, but is still equally painful on the eyes is)
if (false == SomeObject.SomeProperty) DoStuff();
This practice causes a compilation error if you use the assignment operator instead of the equality operator, where in the other, the missing character will be syntactically correct, and have the effect of setting SomeProperty to false the entire expression evaluating to false, regardless of the original value of SomeProperty.
Admin
Well, JS was created long ago to b a simple scripting language and not to much planning went into its design.
Once the problems was note visible you had the option of either trying to fix as much as possible without breaking things, or trying to push a new language onto the browser world.
Both microsoft (with vbscript) and google 8with dart) has tried the second option without any success, so the first option is the winner, and its still better than to only have == which causes much problems due to a bad decision about coercing.
The fact that every browser had unified on one language, AND have allowed it to evolve, mostly in sync, is a marvel in cooperation I wished to have seen in other aspects of society ;)
Admin
@caffiend: Agreed. Yes, different environments have different ideas of which bit pattern(s) represent(s) true. Which is what I said. I also said that good devs put appropriate marshalling code at the point of interface to ensure all downstream code is insulated from dealing with that interop issue. All downstream code should be able to assume any exposed logical value has the 100% conventional behavior of that language's boolean type. Even if the word "type" is really a stretch to cover C's historical [define 2 magic int values to use as a quasi boolean] approach.
As to
if(false == SomeObject.SomeProperty) DoStuff();
I totally get using the order of operands to catch
==
vs=
mistakes. I've occasionally used the idiom myself in places where necessary. Of course that idiom doesn't catch===
vs==
mistakes so it's not as useful in languages, like the example today, that have===
.But far better to not even have the
==
at all.How about instead:
after of course doing the appropriate interop sanitation of quasi-boolean values at the point of interop. If needed.
Bottom line: needing to have guard clauses all over your code because you're letting ambiguous values propagate all over the place is dumb. Common, but dumb.
Great username BTW.
Admin
Unbelievably I ran into a problem with book weirdness in C++ code. What I needed to do was: let tmp = enum value, change enum to different value, call a method, store tmp back to the enum. After that, the enum was changed. There was no other code changing the enum anywhere. What happened? By mistake, tmp was not an enum but a bool. Assigning anything to a bool stored false and true. The enum has a value of 3, which was stored as true = 1, and restored as 1 instead of 3.
Admin
WTFGuy - Comparing to false is almost always preferred over comparing to true. Of course the conditional does not actually require an explicit comparison at all.