I must set a bool to be passed down from a controller to a view in my MVC solution as well as do something (as part of an if). I thought, in order to keep the code concise, I could kill two birds with one stone here:

if (model.MyBoolean = ([Some condition])) { // Do something }

The language permits it, but is this in any way bad practice? Does it compromise readability?

  • 5
    I'd say yes - putting side-effects in an expressions is bad practice. It could also easily be confused for if (model.MyBoolean == [some condition]) { }
    – Lee
    Feb 21, 2017 at 17:06
  • 2
    Just as a side comment here - you don't really mean anti-pattern. Anti-patterns, like patterns, are large scale design elements that are commonly repeated despite having known negative effects. What you're asking is about small scale idiomatic coding, and I'd suggest a more useful term is bad style
    – Jules
    Feb 22, 2017 at 11:12
  • @Jules this may no longer be a pattern, but it used to be. I know some old heads who used to deal with people writing code like this. There's a reason it became known as an anti-pattern. The same old heads who taught me to write C if statements as if (12 == variable) to avoid the headaches this style will cause.
    – RubberDuck
    Feb 22, 2017 at 22:06
  • @rubberduck ... You're missing my point. This isn't a pattern (and never had been) because patterns are about large scale design decisions, not matters of coding style. Patterns are for describing high level design decisions of the kind you might need to describe to a new developer in order to let then get up to speed on your project. This is much lower level than that. The usual name for what you're talking about is an idiom.
    – Jules
    Feb 23, 2017 at 14:16
  • @Jules you're missing my point. This used to be common enough to be a pattern.
    – RubberDuck
    Feb 23, 2017 at 14:17

6 Answers 6


Technically it's OK

There is nothing wrong with using = symbol for both assignment and as an operator (that returns the value of the assignment), and it is certainly legal, and concise. The question is whether it is clear, and that will depend on who is reading it.

Why it is avoided

In my experience, most developers avoid single = in an if statement because it is way to easy to make a mistake, and the mistake is often hard to detect and has a very negative effect. In fact Visual Studio will warn you just because it is such a common problem. I think that is a pretty strong sign you should avoid it.

This is even worse

What's really bad is when the operator is used further along in a short-circuited expression, like this:

if (condition1 | model.MyBoolean = condition2) {}

The above will work, but this will not:

if (condition1 || model.MyBoolean = condition2) {}

If condition1 is false, the assignment won't happen!!! This might not be obvious to someone who is editing the code, and can easily introduce catastrophic logic failure.

What might be OK

On the other hand, using single = like this is not uncommon in certain other constructs, such as in a while loop:

while (bytesRead = streamReader.Read(buffer, x, y) > 0)
    //Do something with the buffer

The above loop will read bytes from a stream and record the length of the bytes read, and will exit the loop if bytesRead is zero.

I have seen this construct all over the place. Because I am familiar with it, it is clear enough to me, and so far hasn't caused me any confusion when I come across it.

Bottom line

You have to decide for yourself whether your code construct is clear. In my opinion (and Microsoft's), it is not, and I wouldn't do it, but perhaps your team is different. Then again it is probably best to err on the side of caution.

I get that you want to be concise. I think most of the posters on this forum think conciseness is far, far less important than clarity. Also, here is a blog post on writing "Really Obvious Code" and why it is a good idea. Your code isn't really obvious.

  • Your "What might be OK" is relatively common in C because of the large number of functions that combine out-parameters with status code return values. Although the assignment is generally put inside parentheses to help bring attention to it and to show that it really is intentional.
    – Kevin
    Feb 22, 2017 at 0:36
  • +1 for use in white conditions, as this is a circumstance where it is often necessary in order to avoid code duplication.
    – Jules
    Feb 22, 2017 at 11:33

While the language allows it, that isn't something that (at least in my experience) gets used a lot. I doubt too many people would get thrown by it, but it will make them have to stop and read more carefully. And in my opinion, I should be able to read the code without having to spend so much mental effort on things like that.

On top of that, if statements aren't a usual place for assignments, so people won't be looking for that to happen there. And for the keen observer, they may think it was a mistake and try to "correct" what is probably a bug.

I would go for something like this:

model.MyBoolean = [condition];
if (model.MyBoolean) { ... }
  • 2
    This. If I got to that line, the first thing that would pass through my mind would be "Hehe, Stupid Interns, forgetting the difference between '= and '=='".
    – T. Sar
    Feb 21, 2017 at 19:32

Because it's very easy to misread = for ==, especially if you're expecting the latter. It's the kind of thing that I'd read, then some time later realize something is amiss, go back to the if and go "Oh, right, that's where it was set".

Also, if you ever need to modify this condition later to something slightly more complex, it's a grand source of bugs - assignment in conditions doesn't play very well with short-circuiting operators!


It looks like you are testing the result of assigning a value to model.MyBoolean. I find this a bit weird to read. Myself, I prefer to read code such as

model.MyBoolean = ([Some Condition]) ;
if (model.MyBoolean)
    /* do stuff */

It might not read as "clever", but I think it's clearer.


It is far more important that code is clear than that it is concise.

I don't see it as clear.

model.MyBoolean = ([some condition]);
if (model.MyBoolean) { // do something}

is clear. It is far less a risk that someone reads it as == in stead of =.

Secondly you have to rely on an assignment always return the assigned value, not only in the current language but also if you port to another language. You might have a language where an assignment always returns true.


I always turn on all reasonable warnings in a compiler, and turn warnings into errors, so my code will be free of warnings.

"All reasonable warnings" with one compiler that I use will warn for "if (x = y)" because it might be an unwanted assignment instead of a comparison. It will not warn for "if ((x = y))" - the unusual extra parentheses tell the compiler that this code isn't written that way by accident but intentional, so no warning.

On the other hand, if you want to write this in one line, you can always write "if ((x = y) != 0)" or "if ((x = y) == true)".

  • +1 for comparison to true. This is, I think, the one way that makes it immediately obvious what's going on.
    – Jules
    Feb 22, 2017 at 11:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.