Variable assignment in an "if" condition

Question

I recently just lost some time figuring out a bug in my code which was caused by a typo:

if (a=b)

instead of:

if (a==b)

I was wondering if there is any particular case you would want to assign a value to a variable in a if statement, or if not, why doesn't the compiler throw a warning or an error?

Answer 1

if (Derived* derived = dynamic_cast<Derived*>(base)) {
   // do stuff with `derived`
}

Though this is oft cited as an anti-pattern ("use virtual dispatch!"), sometimes the Derived type has functionality that the Base simply does not (and, consequently, distinct functions), and this is a good way to switch on that semantic difference.

Answer 2

Here is some history on the syntax in question.

In classical C, error handling was frequently done by writing something like:

int error;
...
if(error = foo()) {
    printf("An error occurred: %s\nBailing out.\n", strerror(error));
    abort();
}

Or, whenever there was a function call that might return a null pointer, the idiom was used the other way round:

Bar* myBar;
... // In old C variables had to be declared at the start of the scope
if(myBar = getBar()) {
    // Do something with myBar
}

However, this syntax is dangerously close to

if(myValue == bar()) ...

which is why many people consider the assignment inside a condition bad style, and compilers started to warn about it (at least with -Wall). Some compilers allow avoiding this warning by adding an extra set of parentheses:

if((myBar = getBar())) {  // It tells the compiler: Yes, I really want to do that assignment!

However, this is ugly and somewhat of a hack, so it's better avoid writing such code.

Then C99 came around, allowing you to mix definitions and statements, so many developers would frequently write something like

Bar* myBar = getBar();
if(myBar) {

which does feel awkward. This is why the newest standard allows definitions inside conditions, to provide a short, elegant way to do this:

if(Bar* myBar = getBar()) {

There isn't any danger in this statement anymore. You explicitly give the variable a type, obviously wanting it to be initialized. It also avoids the extra line to define the variable, which is nice. But most importantly, the compiler can now easily catch this sort of bug:

if(Bar* myBar = getBar()) {
    ...
}
foo(myBar->baz);  // Compiler error
myBar->foo();     // Compiler error

Without the variable definition inside the if statement, this condition would not be detectable.

To make a long answer short: The syntax in you question is the product of old C's simplicity and power, but it is evil, so compilers can warn about it. Since it is also a very useful way to express a common problem, there is now a very concise, bug robust way to achieve the same behavior. And there are a lot of good, possible uses for it.

Answer 3

The assignment operator returns the value of the assigned value. So, I might use it in a situation like this:

if (x = getMyNumber())

I assign x to be the value returned by getMyNumber and I check if it's not zero.

Avoid doing that. I gave you an example just to help you understand this.

To avoid such bugs up to some extent, one should write the if condition as if(NULL == ptr) instead of if (ptr == NULL). Because when you misspell the equality check operator == as operator =, the compiler will throw an lvalue error with if (NULL = ptr), but if (res = NULL) passed by the compiler (which is not what you mean) and remain a bug in code for runtime.

One should also read Criticism regarding this kind of code.

Answer 4

why doesn't the compiler throw a warning

Some compilers will generate warnings for suspicious assignments in a conditional expression, though you usually have to enable the warning explicitly.

For example, in Visual C++, you have to enable C4706 (or level 4 warnings in general). I generally turn on as many warnings as I can and make the code more explicit in order to avoid false positives. For example, if I really wanted to do this:

if (x = Foo()) { ... }

Then I'd write it as:

if ((x = Foo()) != 0) { ... }

The compiler sees the explicit test and assumes that the assignment was intentional, so you don't get a false positive warning here.

The only drawback with this approach is that you can't use it when the variable is declared in the condition. That is, you cannot rewrite:

if (int x = Foo()) { ... }

as

if ((int x = Foo()) != 0) { ... }

Syntactically, that doesn't work. So you either have to disable the warning, or compromise on how tightly you scope x.

C++17 added the ability to have an init-statement in the condition for an if statement (p0305r1), which solves this problem nicely (for kind of comparison, not just != 0).

if (x = Foo(); x != 0) { ... }

Furthermore, if you want, you can limit the scope of x to just the if statement:

if (int x = Foo(); x != 0) { /* x in scope */ ... }
// x out of scope

Answer 5

In C++17, one can use:

if (<initialize> ; <conditional_expression>) { <body> }

Similar to a for loop iterator initializer.

Here is an example:

if (Employee employee = GetEmployee(); employee.salary > 100) { ... }

Answer 6

It depends on whether you want to write clean code or not. When C was first being developed, the importance of clean code wasn't fully recognized, and compilers were very simplistic: using nested assignment like this could often result in faster code. Today, I can't think of any case where a good programmer would do it. It just makes the code less readable and more difficult to maintain.

Answer 7

Suppose you want to check several conditions in a single if, and if any one of the conditions is true, you'd like to generate an error message. If you want to include in your error message which specific condition caused the error, you could do the following:

std::string e;
if( myMap[e = "ab"].isNotValid() ||
    myMap[e = "cd"].isNotValid() ||
    myMap[e = "ef"].isNotValid() )
{
    // Here, 'e' has the key for which the validation failed
}

So if the second condition is the one that evaluates to true, e will be equal to "cd". This is due to the short-circuit behaviour of || which is mandated by the standard (unless overloaded). See this answer for more details on short-circuiting.

Answer 8

Doing assignment in an if is a fairly common thing, though it's also common that people do it by accident.

The usual pattern is:

if (int x = expensive_function_call())
{
  // ...do things with x
}

The anti-pattern is where you're mistakenly assigning to things:

if (x = 1)
{
  // Always true
}
else
{
  // Never happens
}

You can avoid this to a degree by putting your constants or const values first, so your compiler will throw an error:

if (1 = x)
{
  // Compiler error, can't assign to 1
}

= vs. == is something you'll need to develop an eye for. I usually put whitespace around the operator so it's more obvious which operation is being performed, as longname=longername looks a lot like longname==longername at a glance, but = and == on their own are obviously different.

Answer 9

a quite common case. use

if (0 == exit_value)

instead of

if (exit_value == 0)

this kind of typo will cause compile error