The reversed-comparison trick: an argument against

It's a common recommendation to write comparison between variables and constants by putting the constant first. I.e., instead of:

Code:

---

if(x == 1)

---

It's better to write:

Code:

---

if(1 == x)

---

The argument is that if you accidentally type "=" instead of "==", the latter code will fail to compile, while the former will silently be wrong. I want to argue against this technique and open the matter for discussion.

My argument is very simple, and it goes like this. Writing the comparison in "reverse" requires a mental effort, because the mental process usually goes, "If x equals one..." not "if one equals x..." So, you need to remember to do something special when you make a comparison.

But if you have to remember to do something special, why not just remember to check if you've used the correct operator? This is what I do. Whenever I write a comparison, I check to make sure I haven't accidentally typed "=" instead of "==." It requires the same mental effort as reversing the comparison, while making the code a bit more natural.

Why is this superior? Because it works when you're comparing two variables as well. What if you wrote this:

Code:

---

int x, y;
if(x = y) // Oops!

---

Reversing the order will not help you catch a mistake in this case (and it's a very common case). However, the rule "Always check if you've used the proper operator" WILL catch the mistake. It covers all cases. In other words, the reverse-comparison rule is a cop-out -- it encourages you to be lazy and not check if you've used the proper operator. The problem is, it only works some of the time, while my rule works all of the time.

Now, argue with me.

Oh I agree with you, and so does the c.l.c FAQ - http://c-faq.com/style/revtest.html

It's just another one of those dumb 1980's programming tricks which won't go away.
As you say, it's only useful with lvalue == const
A non-lvalue doesn't matter, say if ( strcmp(a,b) = 0 ) will break anyway, without having to go round reducing readability.
And of course where both things could be l-values, the whole edifice crumbles.

Using a decent compiler which can actually warn of all instances of using = in place of ==, and not just reporting syntax error for a few special cases would be a start., But while the whole Indian sub-continent still mandates TurboC (to some level) as the preferred compiler, we'll have to live with the regurgitation from time to time.

Wouldn't it be much simpler and safer to just always use the highest warning level and let the compiler tell you when you did something stupid like use = instead of ==?

Unfortunately I must adhere to this standard at work even though I do not agree with it. The main reason I don't agree with it is because programmers get confused when to use it and when not to. Often times I see this construct used for <,>,<=,>= which then makes the if statement extremely hard to read and understand. I've also seen code where the construct is used in a portion of the if conditonal but not in every conditional of the if. This gets very confusing.

Code:

---

int x = 5;
int value = 2;

if ( (3 == value) && (5 > x) && (20 < x) )
{
  ...
}

---

This is hideous and hard to read. Even though a standard might say specifically to only use the construct on ==, it is often widely misused and applied to other operators which is just terrible. I have even suggested its removal from our standards to the standards committee but to no avail. We also must use warning level 4 which I believe catches this so I'm not sure what the point is.

Quote:

---

Originally Posted by Bubba

Code:

---

int x = 5;
int value = 2;

if ( (value = 3) && (5 > x) && (20 < x) )
{
  ...
}

---

We also must use warning level 4 which I believe catches this so I'm not sure what the point is.

---

Well, it doesn't catch the error I just edited into your code snippet. The warning, as implemented by GCC and MSVC, is only emitted if an assignment is within an if condition, but not part of a parenthesis expression.

That said, I also think that this style is hideous, and the guideline is bad. I also think that warning level 4 in MS's compiler is a mess of useless warnings that are 90% false positives.

Quote:

---

Originally Posted by Bubba

Unfortunately I must adhere to this standard at work even though I do not agree with it.

---

Well at least you actually have standards at work. My place is a chaotic blend of everyone's personal preference.

Quote:

---

Well at least you actually have standards at work. My place is a chaotic blend of everyone's personal preference.

---

I should say 'working towards standards' rather than currently have standards. It's definitely a work in progress at the current time. They are a good thing but it will be hard to get the existing code up to the standards being proposed. Will be an ongoing process as new projects touch portions of the older code.

Quote:

---

Originally Posted by cpjust

Wouldn't it be much simpler and safer to just always use the highest warning level and let the compiler tell you when you did something stupid like use = instead of ==?

---

Like I just posted in another thread...

I always get plenty of

warning C4706: assignment within conditional expression

when I do something like

Code:

---

if (!(lf = fopen(LOGFILE_NAME, "a"))) {
    printf("Failure to create or open logfile: %s\n", LOGFILE_NAME);
    ...
    }


---

When it's exactly what I want to say. But then I always use warning levels = 4 (highest).

So no, the compiler does not tell me when I did something stupid. I wish it did.

brewbuck, I agree with your assertion. It's always better to just check your code. I seem to be able to catch other people's errors quickly by simply reading their code. I read thoroughly. Every operator. I count brackets, double check semicolons, etc. I'm "old school" programmer that lived through needing to spend a half hour rereading code because the line-up for the punch-card reader was so long that it made more sense to re-check the logic while checking for syntax problems. I enjoy lying down and reading printed-out code... if it's particularly elegant. I sometimes find ways to improve it just by staring at it long enough.

Quote:

---

Originally Posted by nonoob

Like I just posted in another thread...

I always get plenty of

warning C4706: assignment within conditional expression

when I do something like

Code:

---

if (!(lf = fopen(LOGFILE_NAME, "a"))) {
    printf("Failure to create or open logfile: %s\n", LOGFILE_NAME);
    ...
    }


---

When it's exactly what I want to say. But then I always use warning levels = 4 (highest).

So no, the compiler does not tell me when I did something stupid. I wish it did.

---

Like I said in your other thread, try using this:

Code:

---

if ( (lf = fopen(LOGFILE_NAME, "a")) == 0 )

---

I bet that'll shut up the warning.