C# if statement. inner workings Q

I've just come across this code snippet in some Exchange 2010 code and I was wondering if anyone knew why the programmer has done it this way. I've never seen an If statement formatted like this. It seems so backwards there must be a good reason for it??

if (true == MsgItem.HasAttachments)
{
    // Code
}

I'm assuming it might have some optimisation over the various other ways o开发者_高级运维f coding the same thing;

if (MsgItem.HasAttachments) 
{
    // Code
}

or

if (MsgItem.HasAttachments == true)
{
    // Code
}

Its not a big deal I'm just curious.

Thanks, Mike

UPDATE: Thanks for all the interesting points raised. Summary seems to be it's down to legacy coding standards.

It is a left-over habit from C or C++ where you might accidentally do:

if (i = 1) {
}

This accidentally assigns 1 to i instead of doing a comparison.

Reversing the order in the check prevents this common mistake.

if (1 = i) {
}

That will not compile, and it is easy to see that you mistakenly typed = instead of ==.

This is still relevant for bools in C# because the value of an assignment is the value that was assigned.

Therefor:

if (SomeBoolVar = true) {
}

Will always be true because you are storing true into SomeBoolVar which evaluates to true. A little mistake that can be hard to track down.

C++ and C programmers sometimes put constants first, to avoid accidentally writing

if (variable = constant)

which would perform an assignment.

In particular, explicitly for boolean comparisons, assuming HasAttachments is writable, this statement would compile and run with no warnings¹, but not have the intended behaviour in C#:

if (MsgItem.HasAttachments = true)

For non-Boolean expressions, this isn't usually an issue in C# because the condition of an if statement has to be implicitly convertible to bool... and for Boolean expressions, your middle form is preferred anyway:

if (MsgItem.HasAttachments)

I would be amazed if it had any performance impact at all - and probably no impact on the generated IL to start with.

¹ Oops - the MS C# compiler does indeed warn for this:

warning CS0665: Assignment in conditional expression is always constant; did you mean to use == instead of = ?

I haven't tried it under Mono though; that may or may not give a warning. It's certainly valid C# code.

These are called "Yoda Conditions" - the original idea is that if you accidentally type a single =, the compilation will fail because the constant value is on the left hand side.

Because the programmer is an clown :-) The whole point of boolean conditions is that you name them such that they make sense as booleans:

if (MsgItem.HasAttachments)

and this clearly does so I don't know why the coder shot themselves in the foot by testing for equality against true.

Because, in all seriousness, true == MsgItem.HasAttachments is just another boolean, so where do you stop?

if (true == MsgItem.HasAttachments)
if (true == (true == MsgItem.HasAttachments))
if (true == (true == (true == MsgItem.HasAttachments)))
: : :

And so on, ad infinitum.

The whole trick of using the constant first to avoid the problem of accidentally using = assignment instead of == equality shouldn't be an issue here since you shouldn't be checking a boolean against either true or false. You should be using one of:

if (cond)
if (!cond)

There's a perfectly fine reason to write if (true == myvar): the variable (or expression) might not be a boolean expression - it might be a bool?. The expression then would be equivalent to if(myvar ?? false) - and I prefer this second syntax since it's immediately obvious that you're dealing with a nullable value. It's technically possible a custom == operator is being used (potentially in combination with an implicit type-conversion), but that's quite unlikely as doing something like that is generally frowned upon and isn't practical or handy either.

Placing the constant first, as many others have pointed out, may reduce accidental errors - personally, I don't think those errors occur often enough to matter, but it can't harm.

Most likely, however, the original author was just a bit confused the moment he wrote that code.

Under mono, they all generate the same IL (ldloc.0; brfalse <LOC>):

Source:

using System;
class Foo
{
    static void Main()
    {
        bool bar = Console.ReadLine() == "bar";
        if (bar)
        {
            Console.WriteLine("bar");
        }

        if (true == bar)
        {
            Console.WriteLine("true == bar");
        }

        if (bar == true)
        {
            Console.WriteLine("bar == true");
        }
    }
}

IL:

// snip...
IL_000a:  call bool string::op_Equality(string, string)
IL_000f:  stloc.0 
IL_0010:  ldloc.0 
IL_0011:  brfalse IL_0020

IL_0016:  ldstr "bar"
IL_001b:  call void class [mscorlib]System.Console::WriteLine(string)
IL_0020:  ldloc.0 
IL_0021:  brfalse IL_0030

IL_0026:  ldstr "true == bar"
IL_002b:  call void class [mscorlib]System.Console::WriteLine(string)
IL_0030:  ldloc.0 
IL_0031:  brfalse IL_0040
// snip...

Personally, it drive me nuts when people do if (SomePredicate() == true). Unless it's actually returning an object with an overloaded == operator, the comparison against true is completely unnecessary. Why stop at one comparison?

// make extra sure SomePredicate returns true
if ((SomePredicate() == true) == true) 
{
     // ...
} 

maybe the == operator has been overloaded and MsgItem.HasAttachments has more information then just a bool? I don't know. Just an idea.

This is common requirement when writing mission critical code. It is there to prevent accidental assignment when you meant to do a check. In a language like C# the compiler will warn you about it, so it's not as necessary, but it is a left over habit for many programmers.