Code Answer: Inadvertent use of = instead of ==

It seems that

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

instead of

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

is a root of many evils.

Why don't all compilers mark it as error instead of a configurable warning?

I'm interested in finding out cases where the construct if (x=y) is useful.

From stackoverflow

one useful construct is for example:
```
char *pBuffer;
if (pBuffer = malloc(100))
{
    //continue to work here
}
```
edit:
as mentioned before, and downvoted several times now, I might add this is not specially good style, but seen often enough to say it's useful. I've also seen this with new, but it makes more pain in my chest.

another example, less controversial, might be:
```
while (pointer = getNextElement(context))
{
    //go for it, use the pointer to the new segment of data
}
```
which implies that the function getNextElement() returns NULL when there is no next element so that the loop is exited.

Learning : Should'nt one be doing this instead? : pBuffer = malloc(100 * sizeof(char)); if(ip == NULL) { printf("out of memory\n"); exit or return }

csl : Any seasoned programmer knows about the explicit test for 0/NULL, but you could do this to clarify: if ( (p = malloc(100*...)) != NULL )

lImbus : @Learning: In the nowadays most unusual case where you are really out of memory, where do you think printf() and following calls will get memory from to bail out with the mentioned error message ?

S.Lott : -1: not a good habit.

BCS : people do it so the compiler need to support it. Even if it is not a good idea.

BCS : @llmbus: now days for a malloc to fail it would normally need to ask for a huge block of ram, I don't think printf needs anywhere near "huge"

peterchen : @S.Lott: there are worse, and there are ziggatons of existing code that do so.
Because it's not illegal (in C or C++ anyway) and sometimes useful...
```
if ( (x = read(blah)) > 0)
{
    // now you know how many bits/bytes/whatever were read
    // and can use that info. Esp. if you know, say 30 bytes
    // are coming but only got 10
}
```
Most compilers kick up a real stink if you don't put parenthesis around the assignment anyway, which I like.

Learning : But this is not the case , the check for if here is ">" and not "=" ? Or am I missing something?

Ed Swangren : he is first assigning to x, which is the same as if (x = 0)

billybob : That's correct. I didn't realise you meant an assignment only rather than simply somewhere within the construct. It would be an interesting grammar that didn't allow this though :-)
Many compiler will detect this and warn you, but only if you the warning level is set high enough.

For example:
```
 ~> gcc -c -Wall foo.c
foo.c: In function ‘foo’:
foo.c:5: warning: suggest parentheses around assignment used as truth value
```
Learning : Where is the if construct in this?
you can use
```
if (0=x)
```
instead, which will cause a compilation error.

Learning : I agree but I still fail to understand why compilers would allow = construct to be legal for an if check.

Ed Swangren : what about while(*str_p1 = *str_p2)? That is a string copy and is perfectly legitimate.

Ed Swangren : Oops... while (*str_p1++ = *str_p2++)
Most of the time, compilers try very hard to remain backward compatible.

Changing their behavior in this matter to throw errors will break existing legit code, and even starting to throw warnings about it will cause problems with automatic systems that keep track of code by automatically compiling it and checking for errors and warnings.

This is an evil we're pretty much stuck with atm, but there are ways to circumvent and reduce the dangers of it.

Example:
```
   void *ptr = calloc(1, sizeof(array));
   if (NULL = ptr) {
       // some error
   }
```
This causes a compilation error.

Learning : ok ... backward compatibility makes sense. But would it not break code which perhaps was already broken to begin with? (I'm assuming that = was used instead of ==) Would'nt that be a good thing?

Ed Swangren : Most of the time those bugs will be found. You wou ld break a lot of code that has no errors.

orip : @Learning - modern compilers issue a warning for `if (x=y)`, which helps

strager : @Learning, In addition, you can tell compilers to treat warnings as errors (which is almost always a good thing!), thus if(x = y) is an error. If you mean to do if(x = y), you can wrap the expression in another set of parentheses: if((x = y)). It's clearer on your intent that way.

jalf : It's not about backwards compatibility, it's about correctness, sticking to how the language is defined. A C++ compiler which flags this as an error *is no longer a C++ compiler*.

Andrew Grimm : I think (Relatively) modern languages like ruby allow x=y in conditionals, so it isn't just backwards compatibility.
It depends on the language.Java flags it as an error as only boolean expressions can be used inside the if parenthesis (and unless the two variables are boolean, in which case the assignment is also a boolean).

In C it is a quite common idiom for testing pointers returned by malloc or if after a fork we are in the parent or child process:
```
if ( x = (X*) malloc( sizeof(X) ) {
   // malloc worked, pointer != 0

if ( pid = fork() ) {
   // parent process as pid != 0
```
C/C++ compilers will warn with a high enough warning level if you ask for it, but it cannot be considered an error as the language allows it. Unless, then again, you ask the compiler to treat warnings as errors.

Whenever comparing with constants, some authors suggest using the test constant == variable so that the compiler will detect if the user forgets the second equality sign.
```
if ( 0 == variable ) {
   // the compiler will complaint if you mistakenly 
   // write =, as you cannot assign to a constant
```
Anyway, you should try to compile with the highest possible warning settings.

strager : +1 for the fork example. I normally keep my malloc on its own line or with the variable declaration, which is much more readable. Then I test using assert or if(x) { ... }.

Learning : +1 for fork. But are'nt we masking an error condition here where fork() throws an error?

R. Bemrose : @Learning: You can still check if pid == -1 inside the if block.

David Rodríguez - dribeas : Yes, if you are in the else part you are guaranteed that the fork worked and you are in the child process. In the else part you still need to check for failure but you have the pid variable for it
That's why it's better to write:

0 == CurrentItem

Instead of:

CurrentItem == 0

so that the compiler warns you if you type = instead of ==.

strager : Not only warns, but throws an error at you. Except in the case when you're comparing to lvalues (e.g. *p == *q).
Is this really such a common error? I learned about it when I learned C myself, and as a teacher I have occasionally warned my students and told them that it is a common error, but I have rarely seen it in real code, even from beginners. Certainly not more often than other operator mistakes, such as for example writing "&&" instead of "||".

So the reason that compilers don't mark it as an error (except for it being perfectly valid code) is perhaps that it isn't the root of very many evils.

Learning : That makes sense. But still begs for the question on why allow it? Which is a genuine case where an = within an if construct makes sense?

Ed Swangren : I think many valid cases have already been posted.

Thomas Padron-McCarthy : I posted an answer using your variables x and y that explains how it works. And if you look at the answer by lImbus, there is a genuine and very common case where it makes sense to use it, and where it is often used.

David Thornley : I've done it. Fortunately, any good compiler will issue a warning.

Ferruccio : ...and it is a perfectly valid and useful C construct.
Simple answer: An assignment operation, such as x=y, has a value, which is the same as the newly assigned value in x. You can use this directly in a comparison, so instead of

x = y; if (x) ...

you can write

if (x = y) ...

It is less code to write(and read), which is sometimes a good thing, but nowadays most people agree that it should be written in some other way to increase readability. For example like this:

if ((x = y) != 0) ...

Here is a realistic example. Assume you want to allocate some memory with malloc, and see if it worked. It can be written step by step like this:

p = malloc(4711); if (p != NULL) printf("Ok!");

The comparison to NULL is redundant, so you can rewrite it like this:

p = malloc(4711); if (p) printf("Ok!");

But since the assignment operation has a value, which can be used, you could put the entire assignment in the if condition:

if (p = malloc(4711)) printf("Ok!");

This does the same thing, but is more concise.

Learning : In your "if (x=y)" example why do you need an if?

Thomas Padron-McCarthy : I need the "if" in my second code snippet if I want it to do the same thing as my first code snippet. The first code snippet contained an "if", so there should be one in the second one too. But I've added another example, with memory allocation, to try to clarify.
You asked why it was useful, but keep questioning examples people are providing. It's useful because it's concise.

Yes, all the examples which use it can be re-written - as longer pieces of code.

strager : ... and as clearer pieces of code, in my opinon. Clarity is more important (for debugging and maintaining) than the number of bytes the source occupies. See golfing competitions. =]

Damien_The_Unbeliever : Ah, but clarity is in the eye of the beholder. The malloc examples show a common idiom.

Damien_The_Unbeliever : In addition, if code is occupying fewer overall lines, then more of the code is visible in one "screenful", which can sometimes aid comprehension.

strager : The malloc "idiom" posted increases the indent level (because you now have another block with a valid alloc'd pointer). In what cases would doing something only if there is memory available (and no problem otherwise) be preferred? I haven't seen this used ever...

Damien_The_Unbeliever : All of the code in the "normal" path will tend to float up and group together under this idiom, whilst making slow progress to the right. As opposed to having 4-5 lines of error handling in the middle of the normal path, to let you forget what's being achieved
There are several tactics to help spot this .. one is ugly, the other is typically a macro. It really depends on how you read your spoken language (left to right, right to left).

For instance:
```
if ((fp = fopen("foo.txt", "r") == NULL))
```
Vs:
```
if (NULL == (fp = fopen(...)))
```
Sometimes it can be easier to read/write (first) what your testing for, which makes it easier to spot an assignment vs a test. Then bring in most comp.lang.c folks that hate this style with a passion.

So, we bring in assert():
```
#include <assert.h>

...
fp = fopen("foo.txt", "r");
assert(fp != NULL);
...
```
when your at the midst, or end of a convoluted set of conditionals, assert() is your friend. In this case, if FP == NULL, an abort() is raised and the line/file of the offending code is conveyed.

So if you oops:
```
if (i = foo)
```
insted of
```
if (i == foo)
```
followed by
```
assert (i > foo + 1)
```
... you'll quickly spot such mistakes.

Hope this helps :)

In short, reversing arguments sometimes helps when debugging .. assert() is your long life friend and can be turned off in compiler flags in production releases.

Tim Post : I should really add, code riddled with asserts() needs to be tested with a -DNDEBUG compiler flag prior to being released .. if only to take out useless calls. This is especially true if you use an inline self made assert() function in lieu of the provided macro, in most implementations.

Paul de Vrieze : You are unfortunately abusing assertions. Assert statements are to detect things that should not happen, and are programming errors. Using assertions to check for success of fopen is broken.

Tim Post : I don't use assertions to check for the success of fopen() :) I check the pointer to the stream for NULL. It was simply an example.
Standard C idiom for iterating:
```
list_elem* curr;
while ( (curr = next_item(list)) != null ) {
  /* ... */
}
```
orip : oops, didn't notice the question was just about an 'if' statement.

Ed Swangren : Same concept really, assignment in a conditional.
The 'if(0 = x)' idiom is next to useless because it doesn't help when both sides are variables ('if(x = y)') and most (all?) of the time you should be using constant variables rather than magic numbers.

Two other reasons I never use this idiom, IMHO it makes code less readable and to be honest I find the single '='to be the root of very little evil. If you test your code thouroughly (which we all do, obviously) this sort of syntax error turns up very quickly.

strager : Completely agree. I mentioned this in my own answer as well.

Ferruccio : In 20+ years of writing C & C++ code this has bitten me only a handful of times and was fairly easy to spot (especially, once I learned to set the warning level to the max).
It is very common with "low level" loop constructs in C/C++, such as with copies:
```
void my_strcpy(char *dst, const char *src)
{
    while((*dst++ = *src++) != '\0') { // Note the use of extra parentheses, and the explicit compare.
        /* DO NOTHING */
    }
}
```
Of course, assignments are very common with for loops:
```
int i;
for(i = 0; i < 42; ++i) {
    printf("%d\n", i);
}
```
I do believe it is easier to read assignments when they are outside of if statements:
```
char *newstring = malloc(strlen(src) * sizeof(char));
if(newstring == NULL) {
    fprintf(stderr, "Out of memory, d00d!  Bailing!\n");
    exit(2);
}

// Versus:

if((newstring = malloc(strlen(src) * sizeof(char))) == NULL) // ew...
```
Make sure the assignment is obvious, thuogh (as with the first two examples). Don't hide it.

As for accidental uses ... that doesn't happen to me much. A common safeguard is to put your variable (lvalues) on the right hand side of the comparison, but that doesn't work well with things like:
```
if(*src == *dst)
```
because both oprands to == are lvalues!

As for compilers ... who can blame 'em? Writing compilers is difficult, and you should be writing perfect programs for the compiler anyway (remember GIGO?). Some compilers (the most well-known for sure) provide built-in lint-style checking, but that certainly isn't required. Some browsers don't validate every byte of HTML and Javascript it's thrown, so why would compilers?

Brian : You can make the while loop even shorter by scrapping the curly braces and adding a semicolon. I prefer that formulation, but some find it confusing.
Try viewing
```
if( life_is_good() )
    enjoy_yourself();
```
as
```
if( tmp = life_is_good() )
    enjoy_yourself();
```
strager : I was about to flame you, but then I realized the usefulness of this. It really helps when things are put into context. However, why would you need to store `tmp` in your example? Perhaps you can adjust it...
In practice I don't do it, but a good tip is to do :

if ( true == $x )

In the case that you leave out an equals, assigning $x to true will obviously return an error.

jalf : It's often a false sense of security though. What if you have if ($x == $y)? How do you reorder that to force an error? The only real answer is to just learn not to make this mistake.

David Thornley : Not to mention that comparing things to true is error-prone. In most languages that don't confine truth to a simple boolean type, there are multiple true values, and only one can be assigned to "true". Consider defining true as 1, and $x is 2, for example.
I think the C and C++ language designers noticed there is no real use in forbidding it because
- Compilers can warn about it if they want anyway
- Disallowing it would add special cases to the language, and would remove a possible feature.
There isn't complexity involved in allowing it. C++ just says that an expression implicitly convertible to bool is required. In C, there are useful cases detailed by other answers. In C++, they go one step further and allowed this one in addition:
```
if(type * t = get_pointer()) {
    // ....
}
```
Which actually limits the scope of t to only the if and its bodies.
I have only had this typo ONCE in my 15 years of development. I would not say it is on the top of my list of things to look out for. I also avoid that construct anyway.

Note also that some compilers (the one I use) issues a warning on that code. Warnings can be treated as errors for any compiler worth its salt. They can also be ignored.
Placing the constant on the left side of a comparison is defensive programming. Sure YOU would never make the silly mistake of forgetting that extra '=' but who knows about the OTHER guy.
The D programming language does flag this as an error. To avoid the problem with wanting to use the value later it allows declarations sort of like C++ allows with for loops.
```
if(int i = some_fn())
{
   another_fn(i);
}
```
Consider augmenting your compiler with static analysis tools that detect such "evil" code, e.g. lint.
As pointed out in other answers, there are cases where using assignment within a condition offers a brief-but-readable piece of code that does what you want. Also, a lot of up-to-date compilers will warn you if they see an assignment where they expect a condition. (If you're a fan of the zero-warnings approach to development, you'll have seen these.)

One habit I've developed that keeps me from getting bitten by this (at least in C-ish languages) is that if one of the two values I'm comparing is a constant (or otherwise not a legal lvalue), I put it on the left-hand side of the comparator: if (5 == x) { whatever(); } Then, if I should accidentally type if (5 = x), the code won't compile.
Part of it has to do with personal style and habits. I am agnostic to reading either if (kConst == x) or if (x == kConst). I don't use the constant on the left because historically I don't make that error and I write code as I would say it or would like to read it. I see this as a personal decision as part of a personal responsibility to being a self-aware, improving engineer. For example, I started analyzing the types of bugs that I was creating and started to re-engineer my habits so as not to make them - similar to constant on the left, just with other things.

That said, compiler warnings, historically, are pretty crappy and even though this problem has been well known for years, I didn't see it in a production compiler until the late 80's. I also found that working on projects that were portable helped clean up my C a great deal, as different compilers and different tastes (ie, warnings) and different subtle semantic differences.
There's a lot of great uses of the assignment operator in a conditional statement, and it'd be a royal pain in the ass to see warnings about each one all the time. What would be nice would be a function in your IDE that let you highlight all the places where assignment has been used instead of an equality check - or - after you write something like this:
```
if (x = y) {
```
then that line blinks a couple of times. Enough to let you know that you've done something not exactly standard, but not so much that it's annoying.
I, personally, consider this the most useful example.

Say that you have a function read() that returns the number of bytes read, and you need to use this in a loop. It's a lot simpler to use
```
while((count = read(foo)) > 0) {
    //Do stuff
}
```
than to try and get the assignment out of the loop head, which would result in things like
```
while(1) {
    count = read(foo);
    if(!(count > 0))
        break;
    //...
}
```
or
```
count = read(foo);
while(count > 0) {
    //...
    count = read(foo);
}
```
The first construct feels awkward, and the second repeats code in an unpleasant way.

Unless, of course, I've missed some brilliant idiom for this...

Paul Stephenson : I'd agree this is the best use of assignment in a conditional, but it doesn't really apply to 'if' statements (as asked by the OP) as well as 'while' loops.

CAdaker : Oops, I forgot it was about if-statements. Then nothing of this is a problem, of course.
About the valid uses of if(i = 0)

The problem is that you're taking the problem upside down. The "if" notation is not about comparing two values like in some other languages.

The C/C++ "if" instruction waits for any expression that will evaluate to either a boolean, or a null/non-null value. This expression can include two values comparison, and/or can be much more complex.

For example, you can have:
```
if(i >> 3)
{
   std::cout << "i is less than 8" << std::endl
}
```
Which proves that, in C/C++, the if expression is not limited to == and =. Anything will do, as long as it can be evaluated as true or false (C++), or zero non-zero (C/C++).

Another C++ valid use:
```
if(MyObject * pObject = dynamic_cast<MyInterface *>(pInterface))
{
   pObject->doSomething() ;
}
```
And these are simple uses of the if expression (note that this can be used, too, in the for loop declaration line). More complex uses do exist.

About advanced uses of if(i = 0) in C++ [Quoted from myself]

After discovering a duplicate of this question at http://stackoverflow.com/questions/3122284/, I decided to complete this answer with an additional bonus, that is, variable injection into a scope, which is possible in C++ because if will evaluate its expression, including a variable declaration, instead of limiting itself to compare two operands like it is done in other languages:

So, quoting from myself:

Another use would be to use what is called C++ Variable Injection. In Java, there is this cool keyword:
```
synchronized(p)
{
   // Now, the Java code is synchronized using p as a mutex
}
```
In C++, you can do it, too. I don't have the exact code in mind (nor the exact DDJ's article where I discovered it), but this simple define should be enough for demonstration purposes:
```
#define synchronized(lock) \
   if (auto_lock lock_##__LINE__(lock))

synchronized(p)
{
   // Now, the C++ code is synchronized using p as a mutex
}
```
This is the same way, mixing injection with if and for declaration, you can declare a primitive foreach macro (if you want an industrial-strength foreach, use boost's).

See the following articles for a less naive, more complete and more robust implementation:
- http://www.drdobbs.com/184401723?pgno=6
- http://www.drdobbs.com/184401728
- http://www.drdobbs.com/184401752
How many errors of this kind really happens?

Rarely. In fact, I have yet to remember one, and I am professional since 8 years. I guess it happened, but then, in 8 years, I did produce a sizeable quantity of bugs. It's just that this kind of bugs did not happen enough to have me remember them in frustration.

In C, you'll have more bugs because of buffer overruns, like :
```
void doSomething(char * p)
{
   strcpy(p, "Hello World, how are you \?\n") ;
}

void doSomethingElse()
{
   char buffer[16] ;
   doSomething(buffer) ;
}
```
In fact, Microsoft was burned so hard because of that they added a warning in Visual C++ 2008 deprecating strcpy !!!

How can you avoid most errors?

The very first "protection" against this error is to "turn around" the expression: As you can't assign a value to a constant, this :
```
if(0 = p) // ERROR : It should have been if(0 == p). WON'T COMPILE !
```
Won't compile.

But I find this quite a poor solution, because it tries to hide behind style what should be a general programming practice, that is : Any variable that is not supposed to change should be constant.

For example, instead of :
```
void doSomething(char * p)
{
   if(p == NULL) // POSSIBLE TYPO ERROR
      return ;

   size_t length = strlen(p) ;

   if(length == 0) // POSSIBLE TYPO ERROR
      printf("\"%s\" length is %i\n", p, length) ;
   else
      printf("the string is empty\n") ;
}
```
Trying to "const" as much variables as possible will make you avoid most typo errors, including those not inside "if" expressions :
```
void doSomething(const char * const p) // CONST ADDED HERE
{
   if(p == NULL) // NO TYPO POSSIBLE
      return ;

   const size_t length = strlen(p) ; // CONST ADDED HERE

   if(length == 0) // NO TYPO POSSIBLE
      printf("\"%s\" length is %i\n", p, length) ;
   else
      printf("the string is empty\n") ;
}
```
Of course, it is not always possible (as some variables do need to change), but I found than most of the variables I use are constants (I keep initializing them once, and then, only reading them).

Conclusion

Usually, I see code using the if(0 == p) notation, but without the const-notation.

To me, it's like having a trash can for recyclables, and another for non-recyclable, and theni n the end, throw them together in the same container.

So, do not parrot a easy style habit hoping it will make your code a lot better. It won't. Use the language constructs as much as possible, which means, in this case, using both the if(0 == p) notation when available, and using of the const keyword as much as possible.

Ferruccio : The worst part is when you start seeing this kind of thing in languages where using = would be a syntax error anyway, such as C# or Python.

paercebal : As a side note, I did swapped to C-like syntaxed languages exactly because of this kind of "free" notation (the "for" notation, to be more exact, so I could be somewhat biased)... ^_^
The assignment as conditional is legal C and C++, and any compiler that doesn't permit it isn't a real C or C++ compiler. I would hope that any modern language not designed to be explicitly compatible with C (as C++ was) would consider it an error.

There are cases where this allows concise expressions, such as the idiomatic while (*dest++ = *src++); to copy a string in C, but overall it's not very useful, and I consider it a mistake in language design. It is, in my experience, easy to make this mistake, and hard to spot when the compiler doesn't issue a warning.
The compiler won't flag it as an error because it is valid C/C++. But what you can do (at least with Visual C++) is turn up the warning level so that it flags it as a warning and then tell the compiler to treat warnings as errors. This is a good practice anyway so that developers don't ignore warnings.

If you had actually meant = instead of == then you need to be more explicit about it. e.g.
```
if ((x = y) != 0)
```
Theoretically, you're supposed to be able to do this:
```
if ((x = y))
```
to override the warning, but that doesn't seem to always work.
```
 if ((k==1) || (k==2)) is a conditional
 if ((k=1)  || (k=2) ) is BOTH a conditional AND an assignment statement
```
- Here's the explanation *
Like most languages, C works inner-most to outermost in order by operator precedence.

First it tries to set k to 1, and succeeds.
```
Result: k = 1 and Boolean = 'true'
```
Next: it sets k to 2, and succeeds.
```
Result: k = 2 and Boolean = 'true'
```
Next: it evaluates (true || true)
```
Result: k still = 2, and Boolean = true
```
Finally, it then resolves the conditional: If (true)
```
  Result: k = 2 and the program takes the first branch.
```
In nearly 30 years of programming I have not seen a valid reason for using this construct, though if one exists it probably has to do with a need to deliberately obfuscate your code.

When one of our new people has a problem, this is one of the things I look for, right along with not sticking a terminator on a string, and copying a debug statement from one place to another and not changing the '%i to '%s' to match the new field they are dumping.

This is fairly common in our shop because we constantly switch between C and Oracle PL/SQL; if( k = 1) IS the correct syntax in PL/SQL.
You may use Yoda Conditionals:

However, they are considered, by many, an ugly and unreadable way, because it violates the "global scope to local scope rule", considered beneficial to a human parser.

Nullstr1ng : 5 is equal to count i see

RegEx sample

RegEx r;

if(((r = new RegEx("\w*)).IsMatch()) {
   // ... do something here
}
else if((r = new RegEx("\d*")).IsMatch()) {
   // ... do something here
}

asign a value test

int i = 0;
if((i = 1) == 1) {
   // 1 is equal to i that was assigned to a int value 1
}
else {
   // ?
}

Code Answer

Monday, March 7, 2011

Inadvertent use of = instead of ==

About the valid uses of if(i = 0)

About advanced uses of if(i = 0) in C++ [Quoted from myself]

How many errors of this kind really happens?

How can you avoid most errors?

Conclusion

0 comments:

Post a Comment

Blog Archive