Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

In C and C++, it is very easy to write the following code with a serious error.

char responseChar = getchar();
int confirmExit = 'y' == tolower(responseChar);
if (confirmExit = 1)

The error is that the if statement should have been:

if (confirmExit == 1)

As coded, it will exit every time, because the assignment of the confirmExit variable occurs, then confirmExit is used as the result of the expression.

Are there good ways to prevent this kind of error?

share|improve this question
Yes. Turn on compiler warnings. Treat warnings as errors and it is not an issue. – Loki Astari Aug 25 '12 at 6:39
possible duplicate of Doesn't "if (0 == value) ..." do more harm than good? – Loki Astari Aug 25 '12 at 7:05
In the given example, the solution is easy. You assign it a boolean value, thus use it as a boolean: if (confirmExit). – Secure Aug 25 '12 at 7:16
The problem is that the mistake was made by the C language "designers", when they chose to use = for the assignment operator and == for equality comparison. ALGOL used :=, because they specifically wanted to use = for equality comparison, and PASCAL and Ada followed the ALGOL decision. (It is worth noting that, when DoD solicited a C-based entry into the DoD1 bake-off, that eventually yielded Ada, Bell Labs declined, saying that "C was not now and would not ever be robust enough for DoD mission-critical software." One wishes that DoD and the contractors had listened to Bell Labs on this.) – John R. Strohm Aug 25 '12 at 12:57
@John, the choice of symbols isn't the problem, it's the fact that assignments are also an expression that returns the assigned value, allowing either a = b or a == b inside a conditional. – Karl Bielefeldt Aug 25 '12 at 16:46
up vote 55 down vote accepted

The best technique is to increase the warning level of your compiler. It will then warn you about potential assignment in the if conditional.

Make sure you compile your code with zero warnings (which you should be doing anyway). If you want to be pedantic then set your compiler to treat warnings as errors.

Using Yoda conditionals (putting the constant on the left hand side) was another technique that was popular about a decade ago. But they make the code harder to read (and thus maintain because of the unnatural why they read (unless you are Yoda)) and provide no greater benefit than increasing the warning level (which also has extra benefits of more warnings).

Warnings are really logical errors in the code and should be corrected.

share|improve this answer
Who's Yoda ? I followed the links from your comment and no one explains this. Was (s)he a person ? Anyway, +1 :) . EDIT: Aaaaa, it was that stupid creature from Star Wars movies... as I didn't see them, I didn't know :D . – Radu Murzea Aug 25 '12 at 8:25
Definitely go with the warnings, not the Yoda conditionals - you can forget to do the conditional constant first as easily as you can forget to do ==. – Michael Kohne Aug 25 '12 at 12:00
I got a pleasant surprise that the most voted answer for this question is 'turn compiler warnings into errors'. I was expecting the if (0 == ret) horror. – James Aug 25 '12 at 21:17
@James: ... not to mention it will not work for a == b !! – Emilio Garavaglia Aug 26 '12 at 21:30
@EmilioGaravaglia: There's an easy workaround for that: Just write 0==a && 0==b || 1==a && 1==b || 2==a && 2==b || ... (repeat for all possible values). Don't forget the obligatory ...|| 22==a && 22==b || 23==a && 24==b || 25==a && 25==b ||... error, or the maintenance programmers won't have any fun. – user281377 Nov 6 '12 at 12:16

You could always do something radical like testing your software. I don't even mean automated unit tests, just the tests every single experienced developer does out of habit by running his new code twice, once confirming the exit and once not. That's the reason most programmers consider it a non-issue.

share|improve this answer
Easy to do when the behaviour of your program is completely deterministic. – James Aug 25 '12 at 21:19
This particular issue can be hard to test for. I've seen people bitten by rc=MethodThatRarelyFails(); if(rc = SUCCESS){ more than once, especially if the method fails only in conditions that are hard to test for. – Steven Burnap Aug 25 '12 at 23:50
@StevenBurnap: That's what mock objects are for. Proper testing includes testing of failure modes. – Jan Hudec Nov 6 '12 at 13:47
This is the correct answer. – Lightness Races in Orbit Jan 3 at 23:23

A traditional way to prevent the incorrect use of assignments within expression is to place the constant on the left and the variable on the right.

if (confirmExit = 1)  // Unsafe

if (1=confirmExit)    // Safe and detected at compile time.

The compiler will report an error for the illegal assignment to a constant similar to the following.

.\confirmExit\main.cpp:15: error: C2106: '=' : left operand must be l-value

The revised if condition would be:

  if (1==confirmExit)    

As shown by comments below, this is considered by many to be an inappropriate method.

share|improve this answer
It should be noted that doing things this way will make you fairly unpopular. – Stargazer712 Aug 25 '12 at 18:03
Please don't recommend this practice. – James Aug 25 '12 at 21:18
It also doesn't work if you need to compare two variables to each other. – dan04 Aug 27 '12 at 2:36
Some languages actually allow assigning 1 to a new value (yes, I know the Q is a bout c/c++) – Matsemann Nov 7 '12 at 9:32

I agree with everyone saying "compiler warnings" but I want to add another technique: Code reviews. If you have a policy of reviewing all code that gets committed, preferably before it's committed, then it's likely this kind of thing will be caught during review.

share|improve this answer

First, raising your warning levels never hurts.

If you do not want your conditional to test the result of an assignment within the if statement itself, then having worked with a lot of C and C++ programmers over the years, and having never heard that comparing the constant first if(1 == val) was a bad thing, you could try that construct.

If your project leader approves of your doing this, don't worry about what other people think. The real proof is whether you or someone else can make sense of your code months and years from now.

If you intention was to test the result of an assignment, however, then using higher warnings might [probably would have] caught the assignment to a constant.

share|improve this answer
I raise a sceptical eyebrow at any non-beginner programmer who sees a Yoda conditional and doesn't immediately understand it. It's just a flip, not difficult to read, and certainly not as bad as some comments are claiming. – underscore_d Jan 1 at 17:39
@underscore_d At most of my employers, assignments within a conditional were frowned on. The thinking was it was better to separate the assignment from the conditional. The reason for being clearer, even at the expense of another line of code was the sustaining engineering factor. I worked in places that had large code bases and a lot of maintenance backlogged. I fully understand someone might want to assign a value, and branch on the condition resulting from the assignment. I see it being done more often in Perl, and, if the intent is clear, I'll follow the design pattern of the author. – octopusgrabbus Jan 2 at 18:54
I was thinking about Yoda conditionals alone (like your demo here), not with assignment (as in the OP). I don't mind the former but don't much like the latter either. The only form I deliberately use is if ( auto myPtr = dynamic_cast<some_ptr>(testPtr) ) { as it avoids keeping a useless nullptr in scope if the cast fails - which is presumably the reason C++ has this limited ability to assign within a conditional. For the rest, yeah, a definition should get its own line, I'd say - much easier to see at-a-glance and less prone to assorted slips-of-the-mind. – underscore_d Jan 2 at 20:30
@underscore_d Edited answer based on your comments. Good point. – octopusgrabbus Jan 2 at 22:24

Late to the party as ever, but Static Code Analysis is the key here

Most IDEs now provide SCA over and above the syntactic check of the compiler, and other tools are available, including those that implement the MISRA (*) and/or CERT-C guidelines.

Declaration: I am part of the MISRA C working group, but I'm posting in a personal capacity. I'm also independent of any tool vendor

share|improve this answer

Just use left hand assignment, compiler warnings can help but you have to make sure you get the right level otherwise you'll either be swamped with pointless warnings or won't be told ones you want to see.

share|improve this answer
You'd be surprised at how few pointless warnings modern compilers generate. You might not think a warning is important, but in most cases you're just wrong. – Kristof Provost Nov 7 '12 at 12:54
Check out the book, "Writing Solid Code"…. It starts with a great discussion about compilers and how much benefit we could be getting from warning messages and even more extensive static analysis. – DeveloperDon Nov 7 '12 at 14:43
@KristofProvost I've managed to get visual studio to give me 10 copies of the exact same warning fro the same line of code, then when this issue was 'fixed' it resulted in 10 identical warnings about the same line of code saying that the original method was better. – Inverted Llama Nov 8 '12 at 11:09

Your Answer


By posting your answer, you agree to the privacy policy and terms of service.

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