11

Possible Duplicate:
Inadvertent use of = instead of ==

C++ compilers let you know via warnings that you wrote,

if( a = b ) { //...

And that it might be a mistake that you certainly wanted to write:

if( a == b ) { //...

But is there a case where the warning should be ignored, because it's a good way to use this "feature"? I don't see any code clarity reason possible, so is there a case where it’s useful?

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Klaim
  • 67,274
  • 36
  • 133
  • 188

17 Answers17

17

Two possible reasons:

  1. Assign & Check

    The = operator (when not overriden) normally returns the value that it assigned. This is to allow statements such as a=b=c=3. In the context of your question, it also allows you to do something like this:

    bool global;//a global variable
    
    //a function
    int foo(bool x){
    
       //assign the value of x to global
       //if x is equal to true, return 4
       if (global=x)
           return 4;
    
       //otherwise return 3
       return 3;
    }
    

    ...which is equivalent to but shorter than:

    bool global;//a global variable
    
    //a function
    int foo(bool x){
    
       //assign the value of x to global
       global=x;
    
       //if x is equal to true, return 4
       if (global==true)
           return 4;
    
       //otherwise return 3
       return 3;
    }
    

    Also, it should be noted (as stated by Billy ONeal in a comment below) that this can also work when the left-hand argument of the = operator is actually a class with a conversion operator specified for a type which can be coerced (implicitly converted) to a bool. In other words, (a=b) will evaulate to true or false if a is of a type which can be coerced to a boolean value.

    So the following is a similar situation to the above, except the left-hand argument to = is an object and not a bool:

    #include <iostream>
    using namespace std;
    
    class Foo {
    public:
        operator bool (){ return true; }
        Foo(){}
    };
    
    int main(){
        Foo a;
        Foo b;
    
        if (a=b)
            cout<<"true";
        else
            cout<<"false";
    }
    
    //output: true 
    

    Note: At the time of this writing, the code formatting above is bugged. My code (check the source) actually features proper indenting, shift operators and line spacing. The &lt;'s are supposed to be <'s, and there aren't supposed to be enourmous gaps between each line.

  2. Overridden = operator

    Since C++ allows the overriding of operators, sometimes = will be overriden to do something other than what it does with primitive types. In these cases, the performing the = operation on an object could return a boolean (if that's how the = operator was overridden for that object type).

    So the following code would perform the = operation on a with b as an argument. Then it would conditionally execute some code depending on the return value of that operation:

    if (a=b){
       //execute some code
    }
    

    Here, a would have to be an object and b would be of the correct type as defined by the overriding of the = operator for objects of a's type. To learn more about operator overriding, see this wikipedia article which includes C++ examples: Wikipedia article on operator overriding

Johannes Schaub - litb
  • 496,577
  • 130
  • 894
  • 1,212
Cam
  • 14,930
  • 16
  • 77
  • 128
  • As an amendment to "Sign and Check", keep in mind that the type can have an `operator bool()` override. – Billy ONeal Jun 26 '10 at 01:41
  • @Billy ONeal: I'd like to add that to my answer if it's alright - that's a great point. However perhaps it should be its own item in the list rather than an addition to 1 or 2? – Cam Jun 26 '10 at 01:44
  • 6
    I'd argue that the second form (overriding = to return a boolean) is terrible, evil, ugly, and makes you a bad person. – Steven Schlansker Jun 26 '10 at 01:50
  • @Steven Schlansker: Why? 'Returns 0 on failure, 1 on success' seems like a reasonable (albeit simple) way to overload `=`. Not trying to be oppositional, just curious as to why you think that. – Cam Jun 26 '10 at 01:54
  • 3
    Because everyone expects a=b to return the value itself. Imagine trying to write a=b=c=d; when you want to assign a bunch of variables to each other... and then figuring out ten hours of bug-busting later that b=c returned 1 instead of c! It violates the general contract of the = operator. – Steven Schlansker Jun 26 '10 at 02:10
  • @incrediman: Please feel free to add to your answer lol. Put it how you see fit. @Steven: +1 to your comments -- good points. – Billy ONeal Jun 26 '10 at 02:11
  • @Billy: Good stuff - if you're wondering why I asked, I figured you might post it as your own answer instead :) – Cam Jun 26 '10 at 02:50
  • @Steven Schlansker: I agree for the *vast majority* of cases, but there are times where a boolean return value for the `=` operator can be useful. Therefore I disagree with the accross-the-board notion that `operator bool ()` is bad. A counter-example for your argument would be an OOP wrapper class for the `bool` type - having the `=` operator return a `bool` value would be appropriate. – Cam Jun 26 '10 at 03:16
  • I think that falls squarely on the "too much" side of "too much magic" for my tastes. But then again, there's a reason I avoid C++ entirely :-p – Steven Schlansker Jun 26 '10 at 04:00
  • I fixed your code to what obviously was your intention. You were just assigning pointers and boolean-evaluated their addresses – Johannes Schaub - litb Jun 27 '10 at 11:36
  • @Johannes Schlaub - litb: Actually what I meant was for this to be the conditional: `*(a=b)`, but what you've put there is simpler anyways, so I'll leave it that way :) – Cam Jun 27 '10 at 14:44
11
while ( (line = readNextLine()) != EOF) {
    processLine();
}
Steven Schlansker
  • 37,580
  • 14
  • 81
  • 100
  • 18
    That's not really the same sort of situation, though, since you're not directly using the assignment as the test. – Tyler McHenry Jun 26 '10 at 01:32
  • 3
    No, it is indeed the same situation: assignment result as the evaluation expression of the if() statement. – lornova Jun 26 '10 at 01:42
  • 7
    No, it's an assignment result used as the left side of a comparison expression which is used as the evaluation expression of the if statement. It's objectively not the same thing because the compiler will never warn about this form. – Tyler McHenry Jun 26 '10 at 01:46
  • 1
    In my defense, the question wasn't at all specific that he didn't want forms other than the simple assignment. – Steven Schlansker Jun 26 '10 at 01:51
  • 1
    It is still the same situation: an assignment is always a valid expression for the evaluation of conditional statements. – lornova Jun 26 '10 at 02:00
  • 4
    This is not the same situation. The actual if conditional is evaluated with the `!=` comparism. – Justin L. Jun 26 '10 at 02:06
8

You could use to test if a function returned any error:

if (error_no = some_function(...)) {
    // Handle error
}

Assuming that some_function returns the error code in case of an error. Or zero otherwise.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Ed.
  • 1,934
  • 15
  • 13
  • I don't see a reason why you can't put the assignment before the conditional. – Jens Gustedt Jun 26 '10 at 06:57
  • You can put the assignment before but that was not the question. – Ed. Jun 26 '10 at 14:24
  • @Ed: yes, this was the question. it was about whether or not it is a good idea to place it there. you made no cause for a good idea, but you just show a syntactically valid case. it easily can be done differently, with no overhead at all if you have a decent compiler. – Jens Gustedt Jun 26 '10 at 21:17
6

This is a consequence of basic feature of the C language:

The value of an assignment operation is the assigned value itself.

The fact that you can use that "return value" as the condition of an if() statement is incidental.

By the way, this is the same trick that allows this crazy conciseness:

void strcpy(char *s, char *t)
{
    while( *s++ = *t++ );
}

Of course, the while exits when the nullchar in t is reached, but at the same time it is copied to the destination s string.

Whether it is a good idea, usually not, as it reduce code readability and is prone to errors.

lornova
  • 6,667
  • 9
  • 47
  • 74
  • You may wish to consider changing "basic feature of the C language" to "basic feature of the C++ language" since that seems to be the primary focus of the question (although it was also tagged as `c`, so it's up to you). – Cam Jun 26 '10 at 02:03
  • 1
    It IS a feature of the C language, inherited by C++. In my opinion the flaw is in the question, as it seems to suggest that this is a C++ only feature. Therefore, it should be edited the question at least as "C/C++". – lornova Jun 26 '10 at 02:13
  • By that reasoning, you can say "basic feature of B" or whatever stone-age language in a discussion about whatever common concept of a particular language, and ask the questioner about changing all language questions about C or C++ to B. I think this is not eligible. C++ and C, like C and B are different langauges. – Johannes Schaub - litb Jun 27 '10 at 11:44
  • @ Johannes: weak argument, as B is a dead language, while C is still one of the most used languages. Then, when something applies to both C and C++ it is very common to say that it is a C/C++ topic. Only when a topic applies **ONLY** to C++ you call it a C++ topic. – lornova Jun 27 '10 at 12:10
3

Although the construct is perfectly legal syntax and your intent may truly be as shown below, don't leave the "!= 0" part out.

if( (a = b) != 0 ) {
   ...
}

The person looking at the code 6 months, 1 year, 5 years from now, at first glance, is simply going to believe the code contains a "classic bug" written by a junior programmer and will try to "fix" it. The construct above clearly indicates your intent and will be optimized out by the compiler. This would be especially embarrassing if you are that person.

Your other option is to heavily load it with comments. But the above is self-documenting code, which is better.

Lastly, my preference is to do this:

a = b;
if( a != 0 ) {
   ...
}

This is about a clear as the code can get. If there is a performance hit, it is virtually zero.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
manovi
  • 61
  • 2
2

I know that with this syntax you can avoid putting an extra line in your code, but I think it takes away some readability from the code.

This syntax is very useful for things like the one suggested by Steven Schlansker, but using it directly as a condition isn't a good idea.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
willvv
  • 8,439
  • 16
  • 66
  • 101
  • +1: It is widely recognised that using any form of "side effect" in a condition is a bad coding style, because it is proven to increase bug rates (every small thing that makes code more complex or less readable, even only by the slightest amount, contributes in this way). This is why you get a compiler WARNING for such code. – Jason Williams Jun 26 '10 at 08:36
2

This isn't actually a deliberate feature of C, but a consequence of two other features:

Assignment returns the assigned value

This is useful for performing multiple assignments, like a = b = 0, or loops like while ((n = getchar()) != EOF).

Numbers and pointers have truth values

C originally didn't have a bool type until the 1999 standard, so it used int to represent Boolean values. Backwards compatibility requires C and C++ to allow non-bool expressions in if, while, and for.

So, if a = b has a value and if is lenient about what values it accepts, then if (a = b) works. But I'd recommend using if ((a = b) != 0) instead to discourage anyone from "fixing" it.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
dan04
  • 87,747
  • 23
  • 163
  • 198
2

A common example where it is useful might be:

do {
 ...
} while (current = current->next);
caf
  • 233,326
  • 40
  • 323
  • 462
2

You should explicitly write the checking statement in a better coding manner, avoiding the assign & check approach. Example:

if ((fp = fopen("filename.txt", "wt")) != NULL) {
    // Do something with fp
}
Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
suyao
  • 23
  • 5
1
while( (l = getline()) != EOF){
        printf("%s\n", l);
}

This is of course the simplest example, and there are lots of times when this is useful. The primary thing to remember is that (a = true) returns true, just as (a = false) returns false.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Aaron Yodaiken
  • 19,163
  • 32
  • 103
  • 184
  • 17
    Never use lowercase L as a variable name. – kenm Jun 26 '10 at 01:23
  • This is what I thought at first, but I don't think this applies to the question. I think in this scenario the compiler doesn't display the warning (I might be wrong though). – willvv Jun 26 '10 at 01:43
  • In C99, such an assignment is much better done with a `for` loop where you declare a local variable for the loop invariant. Only works, though, if you don't need the last line later on in the code. – Jens Gustedt Jun 26 '10 at 06:54
1
void some( int b ) {
    int a = 0;
    if(  a = b ) {
       // or do something with a
       // knowing that is not 0
    } 
    // b remains the same 
 }
OscarRyz
  • 196,001
  • 113
  • 385
  • 569
1

But is there a case where the warning should be ignored because it's a good way to use this "feature"? I don't see any code clarity reason possible so is there a case where its useful?

The warning can be suppressed by placing an extra parentheses around the assignment. That sort of clarifies the programmer's intent. Common cases I've seen that would match the (a = b) case directly would be something like:

if ( (a = expression_with_zero_for_failure) )
{
    // do something with 'a' to avoid having to reevaluate
    // 'expression_with_zero_for_failure' (might be a function call, e.g.)
}
else if ( (a = expression2_with_zero_for_failure) )
{
    // do something with 'a' to avoid having to reevaluate
    // 'expression2_with_zero_for_failure'
}
// etc.

As to whether writing this kind of code is useful enough to justify the common mistakes that beginners (and sometimes even professionals in their worst moments) encounter when using C++, it's difficult to say. It's a legacy inherited from C and Stroustrup and others contributing to the design of C++ might have gone a completely different, safer route had they not tried to make C++ backwards compatible with C as much as possible.

Personally I think it's not worth it. I work in a team and I've encountered this bug several times before. I would have been in favor of disallowing it (requiring parentheses or some other explicit syntax at least or else it's considered a build error) in exchange for lifting the burden of ever encountering these bugs.

stinky472
  • 6,737
  • 28
  • 27
  • +1 I always automatically add the extra parentheses. But _if_ I forget, what compilers will _not_ warn me, given that I always set the warning level quite high? – Joseph Quinsey Jun 29 '10 at 06:53
  • If my vote counted (in my dreams), I'd vote to require it to be a build error without the parentheses or some other explicit syntax in a conditional expression (while/for/if/ternary operator?:). Then we don't have to care about warning levels. – stinky472 Jun 29 '10 at 07:06
1

Preamble

Note that this answer is about C++ (I started writing this answer before the tag "C" was added).

Still, after reading Jens Gustedt's comment, I realized it was not the first time I wrote this kind of answer. Truth is, this question is a duplicate of another, to which I gave the following answer:

Inadvertent use of = instead of ==

So, I'll shamelessly quote myself here to add an important information: if is not about comparison. It's about evaluation.

This difference is very important, because it means anything can be inside the parentheses of a if as long as it can be evaluated to a Boolean. And this is a good thing.

Now, limiting the language by forbidding =, where all other operators are authorized, is a dangerous exception for the language, an exception whose use would be far from certain, and whose drawbacks would be numerous indeed.

For those who are uneasy with the = typo, then there are solutions (see Alternatives below...).

About the valid uses of if(i = 0) [Quoted from myself]

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++).

About valid uses

Back to the non-quoted answer.

The following notation:

if(MyObject * p = findMyObject())
{
   // uses p
}

enables the user to declare and then use p inside the if. It is a syntactic sugar... But an interesting one. For example, imagine the case of an XML DOM-like object whose type is unknown well until runtime, and you need to use RTTI:

void foo(Node * p_p)
{
    if(BodyNode * p = dynamic_cast<BodyNode *>(p_p))
    {
        // this is a <body> node
    }
    else if(SpanNode * p = dynamic_cast<SpanNode *>(p_p))
    {
        // this is a <span> node
    }
    else if(DivNode * p = dynamic_cast<DivNode *>(p_p))
    {
        // this is a <div> node
    }
    // etc.
}

RTTI should not be abused, of course, but this is but one example of this syntactic sugar.

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 Dr. Dobb's Journal'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
}

(Note that this macro is quite primitive, and should not be used as is in production code. The real macro uses a if and a for. See sources below for a more correct implementation).

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).

About your typo problem

Your problem is a typo, and there are multiple ways to limit its frequency in your code. The most important one is to make sure the left-hand-side operand is constant.

For example, this code won't compile for multiple reasons:

if( NULL = b ) // won't compile because it is illegal
               // to assign a value to r-values.

Or even better:

const T a ;

// etc.

if( a = b ) // Won't compile because it is illegal
            // to modify a constant object

This is why in my code, const is one of the most used keyword you'll find. Unless I really want to modify a variable, it is declared const and thus, the compiler protects me from most errors, including the typo error that motivated you to write this question.

But is there a case where the warning should be ignored because it's a good way to use this "feature"? I don't see any code clarity reason possible so is there a case where its useful?

Conclusion

As shown in the examples above, there are multiple valid uses for the feature you used in your question.

My own code is a magnitude cleaner and clearer since I use the code injection enabled by this feature:

void foo()
{
    // some code

    LOCK(mutex)
    {
       // some code protected by a mutex
    }

    FOREACH(char c, MyVectorOfChar)
    {
       // using 'c'
    }
}

... which makes the rare times I was confronted to this typo a negligible price to pay (and I can't remember the last time I wrote this type without being caught by the compiler).

Interesting sources

I finally found the articles I've had read on variable injection. Here we go!!!

Alternatives

If one fears being victim of the =/== typo, then perhaps using a macro could help:

#define EQUALS ==
#define ARE_EQUALS(lhs,rhs) (lhs == rhs)

int main(int argc, char* argv[])
{
   int a = 25 ;
   double b = 25 ;

   if(a EQUALS b)
      std::cout << "equals" << std::endl ;
   else
      std::cout << "NOT equals" << std::endl ;

   if(ARE_EQUALS(a, b))
      std::cout << "equals" << std::endl ;
   else
      std::cout << "NOT equals" << std::endl ;

   return 0 ;
}

This way, one can protect oneself from the typo error, without needing a language limitation (that would cripple language), for a bug that happens rarely (i.e., almost never, as far as I remember it in my code).

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
paercebal
  • 81,378
  • 38
  • 130
  • 159
  • I like how you mix in that foo() example as a blatant violation of LSP. – Edward Strange Jun 26 '10 at 02:52
  • +1, excellent answer. To point out one part I like - note about lhs being a constant for `==` is particularly good advice. – Cam Jun 26 '10 at 05:13
  • Just for the records, the reasons given in the first part only apply to C++, not to C. C99 allows for the declaration of variables only in a `for` statement. For your trick to avoid this common error: well I find constants on the left not easy to read. For me this is a bit as if you were advertising `10[a]` instead of `a[10]` ;-) Frankly, I think the compiler warning is good enough. – Jens Gustedt Jun 26 '10 at 07:03
  • @Noah Roberts: I agree. This is why I wrote "RTTI should not be abused". Sometimes, you just don't have the right tools (visitor, access to objects) to use a more correct OO way. – paercebal Jun 26 '10 at 10:05
  • @Jens Gustedt: When I started this answer (and lost a lot of time trying to track the right Dr Dobbs article on variable injection), the tag was still C++, which explain I don't even considered C here. Still... – paercebal Jun 26 '10 at 10:15
  • @paercebal: sorry, didn't notice that the C label had been added on the fly. – Jens Gustedt Jun 26 '10 at 14:13
  • When thinking of it, for C++, I am not really convinced of your arguments, and even technically you are kind of wrong since you are talking about initialization of variables, and not of assignment. I think the question targets assignment, `operator=` in C++, and my understanding of C++ is that `operator=` and a constructor with one argument are quite distinct. So my idea would be in that case to avoid the `=` completely by using the function-like initialization systematically: don't use `operator=` in conditionals. – Jens Gustedt Jun 26 '10 at 14:22
  • @Jens Gustedt: The main argument here is "In C/C++, the `if` notation is not about comparing two values, it's about evaluating an expression". It is perhaps too much powerful, but an expression can have multiple appearances, and forbidding the `=` in the if just because of some random error is overkill. The fact I declare a variable in the `if` is an example of a valid case where and `=` operator can appear in a `if` (even if it is resolved as through constructor) – paercebal Jun 26 '10 at 18:33
  • You don't give examples for the `operator=` but for constructors. The fact that constructors with one argument can also be called by means of the `=` syntax is an historical corner case of C++. Agreed that `if` is about expressions, but you were not adding for the cause that it is good style to have `operator=` in these expressions. You don't give one example of even a use of it. BTW, even in the macro example you give, you use yourself the function notation. – Jens Gustedt Jun 26 '10 at 21:10
  • When i saw the first two big captions, i so knew the answer is by @paercebal :) – Johannes Schaub - litb Jun 27 '10 at 11:47
  • @Johannes Schaub - litb : Thanks for the compliment... ^_^ ... – paercebal Jun 29 '10 at 15:38
  • @Jens Gustedt: Other people already contributed by giving the examples you want to see. My point is that if the `if` is all about evaluating expressions, then there is no way to remove the `operator=` without crippling the language. And what's the point of crippling the language just because of a typo? Personally, I can't even remember this bug biting me. – paercebal Jun 29 '10 at 15:42
  • @Jens Gustedt: ... And I believe you take the `if(a = b)` too literally. My examples uses the constructor even if writting `=`, Ok, but this is not a corner case of C++. It is how C handles initialization of primitives. The C++ corner case would be the constructor/cast notation. It could be used, of course. But I see no point in forbidding `=` just because some people fear typos, when other dangerous notations, like the C-casts, are still authorized as they are in C++. – paercebal Jun 29 '10 at 15:44
  • @Jens Gustedt: ... Conclusion : My examples were about showing there were more between a `if` parentheses than a simple comparison, and showing that one just couldn't question about the presence of an operator because of its supposed potential for errors, and should instead see the larger picture around it. – paercebal Jun 29 '10 at 15:46
  • @paercebal: The question was not: *should we cripple C++ (or C) to not allow assignment in an `if` expression?* It clearly targeted (in C++ terms) `operator=` and asked whether or not this was a good idea. You just didn't convince me, personally, for the simple reason that you didn't answer the question, but a different one. I still think that it is generally a bad idea to use it, and that the gcc-habit to put extra parenthesis is a good way to make intentions clear. That's it. – Jens Gustedt Jun 29 '10 at 16:04
  • @paercebal i've not really read the content (i'm no good at coding styles), but i know you are the man of using big captions for each chapter of your answers :) So i immediately knew it was yours after reading the first of the two ^^ – Johannes Schaub - litb Jun 29 '10 at 16:19
  • @Jens Gustedt: I understand your viewpoint, but I disagree with your strict limitation of answering about `operator =` because I do believe the subject is larger than that. So I guess we agree on the fact that we disagree... ^_^ ... – paercebal Jun 29 '10 at 18:29
  • @Johannes Schaub - litb: I took it as a compliment/recognition of my answering style... ^_^ ... – paercebal Jun 29 '10 at 18:44
0

Never!

The exceptions cited don't generate the compiler warning. In cases where the compiler generates the warning, it is never a good idea.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Larry Watanabe
  • 10,126
  • 9
  • 43
  • 46
0

There's an aspect of this that hasn't been mentioned: C doesn't prevent you from doing anything it doesn't have to. It doesn't prevent you from doing it because C's job is to give you enough rope to hang yourself by. To not think that it's smarter than you. And it's good at it.

dj_segfault
  • 11,957
  • 4
  • 29
  • 37
0

RegEx sample

RegEx r;

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

Assign a value test

int i = 0;
if((i = 1) == 1) {
   // 1 is equal to i that was assigned to a int value 1
}
else {
   // ?
}
Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
jaysonragasa
  • 1,076
  • 1
  • 20
  • 40
0

My favourite is:

if (CComQIPtr<DerivedClassA> a = BaseClassPtr)
{
...
}
else if (CComQIPtr<DerivedClassB> b = BaseClassPtr)
{
...
}
Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Schmoo
  • 424
  • 4
  • 8