0

I got a headscratcher: someone submitted code to test whether all of some checkboxes were unchecked, and it indicated True when an even number were checked. The code looked something like:

if (box1.checked == box2.checked == box3.checked == box4.checked == box5.checked == false) ...

and I read it naively as: if each one is false... But this was incorrect. I figured out why (C# evaluates from left to right, and the result of a boolean compare is a boolean: false == false evaluates to true), but I wondered if this shows up often, and has a name? I guess I would name it Chained Falsehood or maybe False Decay as it would be fine if all operands were True.

The same idea works fine with assignment, so I can see why the coder tried it this way.

Ok, based on comments, this should not be called an anti-pattern because it is a bug. I think it should be called an idiom from another language. But vanity of vanities, thy name is Python, and its name should be called Haddock's Ayes.

  • 4
    This is my first time seeing something like this. I don't think it's a pattern. Maybe just call it an "anti"? :) – Samuel Sep 27 '17 at 18:57
  • 3
    seems like a bug more than a pattern – yitzih Sep 27 '17 at 19:55
  • @yitzih if more than one person does it, then it's an anti-pattern, right? I have seen something like this before, I just didn't think it through and recall it. –  Sep 27 '17 at 19:58
  • 2
    I don't think the criteria for a pattern (or anti-pattern) is as simple as "more than one person does it". A pattern is more of an established practice for solving a particular type of problem. Just because a few people use it here or there doesn't mean it rises to the level of pattern. We would have millions of patterns if that were the case. – Eric King Sep 27 '17 at 20:01
  • 1
    Note that some languages do support chained comparison operators like this, notably Python and common mathematical notation. This might be a honest mistake from someone who has recently switched languages. – amon Sep 27 '17 at 20:07
  • 1
    I'm not sure what is the intent but I'm pretty sure that is not what the code does... (whatever the intent was)... so if I translate it would yield (((((box1.checked == box2.checked) == box3.checked) == box4.checked) == box5.checked) == false)... What I think the original intent would be !(box1.checked || box2.checked || box3.checked || box4.checked || box5.checked) – Newtopian Sep 27 '17 at 20:11
  • @Newtopian they just wanted to see if all the boxes were 'false', which is what this looks like it does. It ends up coalescing pairs, and changing two false values to a true value along the way. –  Sep 27 '17 at 20:35
  • 1
    yeah... that's what I imagine... definitively a bug then. C# does not support chaining of equality (of anything really)... but it will not complain either. As others said, probably an assumption they carried from a language that does support equality chaining. – Newtopian Sep 27 '17 at 20:43
  • @Newtopian I think it carried over from cases like: *x = y = z = 5;* which works perfectly fine. The false value is what trips it up. It seems similar to Null Propagation. It is a bug... which looks like it should work. I like to give such things names, so that when I explain them, they are memorable. I tend to give all my bugs names, and stick them on a big white chart with pins. They look so nice up there. –  Sep 27 '17 at 20:49
  • 1
    it is a bug in C#... it is perfectly fine in Python and will work as expected... that sais... `DeMorgan's Fallacy`... or perhaps `Chained Switcharoo's`... or `your False are belong to us` – Newtopian Sep 27 '17 at 20:58
  • 4
    @Newtopian It's definitely not a bug in C# except perhaps not warning against/disallowing it. In my opinion, Python's support for this is a misfeature. If nothing else, Python is clearly the outlier here. (In math, there's usually a distinction between propositions and the objects of the language so *x* = *y* = *z* is unambiguous, as (*x* = *y*) = *z* is not even syntactically correct. If we were talking about = as a Boolean operation, i.e. <->, then *x* <-> *y* <-> *z* would behave like the C# if <-> had the appropriate associativity, but it is clearest for it to be non-associative.) – Derek Elkins left SE Sep 27 '17 at 21:13
  • 1
    @Derek I did not mean that it was a bug in the language but rather that in the context of C#, this code is buggy. sorry for the confusion I would admit my statement was not very clear. And yes, python is an outlier here, in fact before this question I was not even aware python actually supported this and always did it explicitly as I would have done in C#. That said, now that I know it does support it... I very likely still would not use this syntax for all the reasons and reactions gathered herein. – Newtopian Sep 27 '17 at 21:18
  • @Newtopian Okay, yes, I definitely misinterpreted your comment then. – Derek Elkins left SE Sep 27 '17 at 21:31
  • Python. This person had previously studied Python, I think. Interesting. –  Sep 27 '17 at 21:34
  • 4
    How about `The Boolean Centipede` in reference to http://www.imdb.com/title/tt1467304/ – Newtopian Sep 27 '17 at 21:42
  • @Newtopian "The horror, the horror." –  Sep 27 '17 at 22:19
  • *"Python is the new BASIC"* has a nice ring to it. –  Sep 27 '17 at 22:27
  • 2
    It is a misleading piece of code that is interpreted as legal by the compiler. However, given that it can be misused (for example, people can start inserting back-doors into various open-source projects by using code like this), it is reasonable to ask the next revision of C# compiler to check for and forbid code like this (when used without parentheses). – rwong Sep 27 '17 at 22:57
  • 2
    @rwong Indeed, what I find strange is why C# apparently assigns an associativity to the relational operators at all. Code like `x == y == z` should simply be treated as an ambiguous use of a non-associative operator and rejected by the compiler for syntactic reasons alone. If you really wanted to, you'd still be able to write `(x == y) == z` which makes it a bit more clear what is going on, and I think it would be less likely to be written by someone who believes `x == y == z` should mean `x == y && y == z`. – Derek Elkins left SE Sep 27 '17 at 23:08
  • I guess it is actually a "dark idiom", created perhaps by an idiom savant? –  Sep 28 '17 at 00:40
  • @nocomprende That's just bad code. A idiom in theory works, this doesn't. Naming it a "Dark Idiom", or using similar "cool words", make it look like a "desirable way" to write something. Black Magic code is an example - the code works, but usually the reason _why_ it works is hidden deep below several interactions of obscure language features, math, or some other sort of exoteric thing. But it _works_, despite being extremely hard to grasp for someone not familiarized with it. The code on the OP example is just a plain bug. – T. Sar Sep 28 '17 at 11:17
  • @T.Sar fine, it is a bug, I just wondered what crack it crawled out of. Python, apparently? I don't know that language, so it would not have occurred to me to write something like this, yet the intent was obvious. Like a "Garden Path sentence", I suppose. This is no garden-variety bug though, it is interesting. –  Sep 28 '17 at 12:10
  • I call this obfuscated code. `a == b == c == d == e == f` is exactly the same as `!a ^ b ^ c ^ d ^ e ^ f` where `^` is the xor operator. It is just written in a more unusual syntax. (it works differently with an even and an odd number of terms) – Florian F Sep 28 '17 at 14:18
  • @FlorianF It would be obfuscated code if the code was working correctly, but it isn't. The intention of the developer wasn't to use a chain of XOR's, but a chain of AND's. The end result ended up not working as expected. – T. Sar Sep 28 '17 at 14:25
  • I can understand the confusion. `a = b = c = false` sets all variables to false. One might (wrongly) assume that the compiler "sees" this and knows to individually **assign** (`=`) the rightmost value to all other operands (somewhat similar to `bool a,b,c;` where the compiler applies the type to all variables). Working under that assumption, you'd then also expect that `a == b == c == false` will be seen by the compiler and it will **compare** (`==`) the rightmost value to all other operands. However, the lack of logical operator (and? or?) should be the first clue that this is not correct. – Flater Sep 28 '17 at 15:37
  • @Flater It is definitely not logical. Intuitive though. Too bad they disagree in this case. –  Sep 28 '17 at 16:11
  • 1
    @T.Sar Sorry I read the post incorrectly. I thought the behaviour that the test passes with an even number of checkboxes checked was the wanted behaviour, while it is in fact the observed behaviour. So it is a mistake. It is called "chained comparison" in python, but is not valid in C#. – Florian F Sep 29 '17 at 07:02

5 Answers5

10

This is just a coding error due to someone's misunderstanding. I don't know why you think it should have a name. It's definitely not an anti-pattern. An anti-pattern describes a common architectural pattern that negatively impacts maintainability, not a coding error.

I don't know how common this is, but I expect it to be rather rare in any professional context even from junior programmers. I can see it being more common in some introductory programming classes, but it should quickly be discovered to be incorrect. First, the students won't be taught this. Second, even the most cursory of testing will reveal that it doesn't work. Third, the most obvious way of doing this is simpler, namely !box1.checked && !box2.checked && ... && !boxN.checked.

If you are working in a professional context (and arguably even in a non-professional context), the real problem is a process anti-pattern where people are submitting untested code.

Derek Elkins left SE
  • 6,591
  • 2
  • 13
  • 21
  • I am a professional in a professional context, teaching introductory programmers. –  Sep 27 '17 at 20:15
  • 2
    @nocomprende: Still doesn't have a name. Call it "bad coding." – Robert Harvey Sep 27 '17 at 20:17
  • @RobertHarvey Off-By-One has a name. –  Sep 27 '17 at 20:19
  • 1
    @nocomprende And you should use this as a teachable moment to illustrate why testing ones code in multiple scenarios is important. If the submitter had done that, you would have never seen this code, so it's not necessary to explicitly "warn against" this misconception (which would probably be a bad idea pedagogically anyway). – Derek Elkins left SE Sep 27 '17 at 20:19
  • 2
    @nocomprende: This isn't off-by-one; it's just bad coding. You can code badly an almost unlimited number of ways; doesn't mean each way deserves a name. – Robert Harvey Sep 27 '17 at 20:21
  • @RobertHarvey how many times must something show up before it gets a name? Can we name it if it is convenient? I thought this one was interesting because it only shows up for false, not true, and it *looks* like it should work. –  Sep 27 '17 at 20:24
  • 1
    @nocomprende: This one is particularly rare. It's such a newbie mistake that you don't ever see it in code written by someone with more than a week's worth of coding experience. Off-by-one errors are common, even occasionally among professionals. – Robert Harvey Sep 27 '17 at 20:25
  • @nocomprende The difference between an off-by-one error and this is that an off-by-one error is not usually due to a misunderstanding of the language, but is usually more like a typo/thinko, i.e. an inadvertent error. Also, off-by-one errors are often harder to catch with testing, though again, such errors are teachable moments for why you should test extreme values and edge cases. (Of course, a better solution is to write code in a way where you aren't doing index manipulation-like things.) – Derek Elkins left SE Sep 27 '17 at 20:25
  • 4
    @nocomprende: Further, I think the current generation of programmers' obsession with naming things is *actively harmful to our profession.* Using arbitrary vocabulary gives the false impression that the people using said vocabulary understand things that they actually do not understand. – Robert Harvey Sep 27 '17 at 20:28
  • I can see the headline now: **Naming considered harmful.** That would solve all our programming problems! –  Sep 28 '17 at 01:11
  • 2
    @nocomprende Except for cache invalidation. – Derek Elkins left SE Sep 28 '17 at 01:16
  • 2
    @RobertHarvey That's so true. Dependency Injection was something _a lot of people_, including myself, were using even before it had a name. When the term popped up everywhere, it left a lot of people scratching heads trying to understand what was so special there. And worse - a few newbies that didn't grasp the concept but liked the fancy name put their hands on some weird API and ended up writing some horrendous pieces of Injection Hell. – T. Sar Sep 28 '17 at 11:14
4

It's not an anti-pattern. To be an anti-pattern, it would first have to be a pattern. Which means it would have to be a common solution to a problem to be a pattern, and a bad common solution to be an anti-pattern. But this is not a common solution to anything.

I'd say it is a WTF. WTF is obviously an abbreviation. You may assume that it means "Worse Than Failure". Or you may assume it means something else.

I have once in my life had to check that at least two of three conditions are true, so I counted how many were true. If I actually needed to do what the code here does (not what it was likely intended to do), I'd write

int falseCount = (box1.checked ? 0 : 1) + (box2.checked ? 0 : 1) etc.
if (falseCount % 2 == 1) { ... }
gnasher729
  • 42,090
  • 4
  • 59
  • 119
3

I see this every now and then, not on this exact form but in the more general form of "a line of code doing a lot of things".

It could be a pattern if common and misguided ways for trying to achieve premature optimization or to make you look smart qualified as patterns (I am not really sure if they do) and it had a concise name. In that case it would be an antipattern because:

  • Premature optimization and looking smart should not be amongst your concerns.
  • More often than not it does not buy you any actual optimization (does not really solve this problem).
  • Makes you look less smart (does not solve this problem either).
  • Makes easier to make mistakes and introduce bugs (just like in your example).
  • Obfuscates your intent.
  • Makes your code harder to debug.
Stop harming Monica
  • 835
  • 1
  • 8
  • 11
  • If I wrote x = y = z = 5; that would make perfect sense, and probably work in most languages, wouldn't it? *x == y == z == false* does not work, because the **pairs** of *false == false* become true. The false values make it fail where all *trues* would succeed. They might have done it to look smart, not sure. I don't think it had to do with optimization. It doesn't obfuscate the intent at all, if anything the intent is more clear than the correct formulations, especially because those use negation. –  Sep 27 '17 at 20:39
  • 1
    @nocomprende Intent being clear is meaningless when the code is wrong – mmathis Sep 27 '17 at 21:04
  • @mmathis now if we could just create a language where clear intent produced correct code... –  Sep 27 '17 at 21:28
  • 1
    @nocomprende I find it hard to read and somewhat obfuscated even if I am used to python. Anyway at this point I am confused about what "this" refers to in "I wondered if this shows up often". Chained comparison? Getting the semantics wrong? Using python idioms in c#? I stand by my claims but it looks like I missed the point of the question. – Stop harming Monica Sep 27 '17 at 21:41
  • 1
    @nocomprende: I see what you mean when you say that the intent is more clear; but you're missing something: how would this notation meaningfully be able to differ between `&&` (all of them need to be false) and `||` (one of them must be false)? The absence of this distinction should reveal that the intent is _incomplete_. – Flater Sep 28 '17 at 15:43
  • @Flater If a bunch of things are equal to each other, and all of them are equal to false, it seems quite clear to me. The problem is that each equality test is considered as a pair, and it produces a result. So there is implied grouping - the good old BEMDAS or whatever people in different countries call it - which is both a blessing and a curse. There are languages with no precedence rules, just left to right and parens. Here the results are 'promoted' and then *they* are compared, which mixes levels. The original line has no levels, and it is only the (implied) rules that trip things up. –  Sep 28 '17 at 16:17
1

I am going to disagree with the other answers here and say this is an anti-pattern as there is a pattern in use. That pattern is using equality to test booleans, rather than using logical operators. eg using if (b == true), rather than if (b). A common excuse offered for using this (anti)pattern is that ! is hard to read, so those folk write if (b == false) rather than if (!b) and then use == true for consistency.

This question neatly highlights the dangers with that pattern though. Using logical operators results in code working as expected, so this is an opportunity to educate folk in the idiomatic use of those logical operators:

// this code works as expected, unlike using ==
if (!(box1.checked || box2.checked || box3.checked || box4.checked || box5.checked)) ...
David Arno
  • 38,972
  • 9
  • 88
  • 121
  • What you just did was isolate each part of the Boolean expression--which is the correct thing to do. In essence you now have 5 separate `boxX.checked` expressions ORed together instead of one like in the OP. I agree that using `box1.checked` is easier to read than `box1.checked == true`, but if each expression were written with the longer form, then your answer would essentially be the same. Just more verbose. – Berin Loritsch Sep 28 '17 at 12:10
  • 1
    What is really fun is when someone inverts the condition to make it more understandable, which is fine, then has an empty Then body and puts the code in an Else body. So amusing. –  Sep 28 '17 at 12:14
  • I do also find `b == true/false` unpleasant and common, but this is focusing on a side issue. If the original code was instead checking if all boxes had the same checked state (i.e. all checked or all unchecked), the original author of the code would presumably have written the same thing except omitting the `== false` at the end. It's completely reasonable to use `==` to test the equality of Booleans in that case, but, of course, this still can't be written as `x == y == z`. – Derek Elkins left SE Sep 28 '17 at 12:21
  • 1
    I'm not sure if using equality to test booleans is a pattern os just coding style. Patterns usually don't apply to single lines of code, they are somewhat more complex constructs that try to solve a problem. Arguing that this is a pattern open doors for claiming that "using guard conditions" is a pattern, too. I'm not _sure_ they are, but I look at them just as good practices, not a high-level problem solving structure. – T. Sar Sep 28 '17 at 14:37
  • @T.Sar yeah, I decided to change from 'pattern' to 'idiom'. This is indeed an idiom (or at least good code) in Python. Careful where you give that thumb-circle OK sign, eh? –  Sep 28 '17 at 16:19
-1

There are reasons why it is considered good practice to separate both assignments and Boolean expressions. The number of surprises you can get from implicit converts (if the language supports that) and such increase with each segment in the chain.

This is the first time I've seen a Boolean chain, and hopefully it is the last. I would consider a special case of "chained assignment", which you can find a lot of conversation about it when you search.

More often than not, chained assignments have unexpected side-effects. Most code style guidelines I've seen recommend avoiding them completely.

Since there needs to be multiple instances of anything to be called a pattern, I'd say this type of thing fails that threshold. However it does win a gold star for creative bug writing.

Berin Loritsch
  • 45,784
  • 7
  • 87
  • 160
  • 6
    There are no assignments, chained or unchained, in the question. – Stop harming Monica Sep 27 '17 at 19:24
  • @Goyo, The same problems with chained assignments are affecting this problem. There is an _implicit_ assignment to the if statement at the end of this. Like I said a special case of a well known problem. – Berin Loritsch Sep 27 '17 at 20:13
  • 1
    @BerinLoritsch What are you talking about? I don't even know what you mean by an "assignment to the `if` statement" implicit or otherwise. It's possible that even at the level of assembly literally nothing gets updated other than the instruction pointer, so I'm not sure what you believe is being implicitly assigned to. – Derek Elkins left SE Sep 27 '17 at 20:35
  • x = y = z = 5 would work. I have not seen cases where that is problematic. But when you start throwing Booleans around, false values do strange things. (True values, of course, do not. It is like odd and even numbers, or primes and composite numbers. The world is just weird.) –  Sep 27 '17 at 20:41
  • x = y = z = false would work just as fine... – Newtopian Sep 27 '17 at 21:25
  • 1
    @Newtopian so what's a few = signs between friends? –  Sep 27 '17 at 21:29
  • @nocomprende There are languages where `x = 1` evaluates to the *previous* value of `x` (or to some unit value), so that chained assignment would (probably) be not what was intended. Also, the code in your question does *not* work if you replace `false` with `true` and correspondingly change the intended outcome. This has nothing to with some distinction between true and false. A conditional expression is not correct when it is true when it should be true. It also has to be false when it should be false! – Derek Elkins left SE Sep 27 '17 at 21:29
  • @DerekElkins I just asked what to call it. –  Sep 27 '17 at 21:33
  • @nocomprende I might be misreading them, but some of your comments start to sound like you believe code similar to that in your question would be correct (though presumably still discouraged) if the test was instead for whether any?/all? the boxes were checked. I don't know what "strange things" you think false values "do" (that true values do not "do"). – Derek Elkins left SE Sep 27 '17 at 21:40
  • @DerekElkins maybe you don't want to know... –  Sep 28 '17 at 01:13