Friday, January 22, 2010

| or ||? (or Or or Or?)

This is part 1 of my new SEO unfriendly series. I will be writing posts with un-searchable headings

I've just finished a code review with a colleague who's 1st language is not c#. As part of his solution there were several lines like this:
if (conditionA | conditionB)
    //dostuff

Without even thinking about it I have always used the || (unless I want to do a bitwise operation).

Lets just clarify what | does. It a bitwise comparison for integral types e.g. (byte, int, etc..) and for bool types it performs a logical OR but checks both operands.

Most languages I've used short circuit the second check if the first check is true. If your condition has side-effects you need to be very careful.

Consider the following program:
namespace OrProgram
{
    class Program
    {
        static void Main(string[] args)
        {
            if (ConditionTrue() | ConditionFalse() )
                Debug.WriteLine("Outcome was True");
            else
                Debug.WriteLine("Outcome was False");

            if (ConditionTrue() || ConditionFalse())
                Debug.WriteLine("Outcome was True");
            else
                Debug.WriteLine("Outcome was False");
        }

        private static bool ConditionFalse()
        {
            Debug.WriteLine("ConditionFalse has been called");
            return false;
        }

        private static bool ConditionTrue()
        {
            Debug.WriteLine("ConditionTrue has been called");
            return true;
        }
    }
}

With the | operator you will get the output:
ConditionTrue has been called
ConditionFalse has been called
Outcome was True

With the || operator you will get:
ConditionTrue has been called
Outcome was True

So if you need to make sure both sides of the operator are called you can use the | operator but it would be much clearer if you did:

var isTrue = ConditionTrue();
var isFalse = ConditionFalse();

if (isTrue || isFalse)
    Debug.WriteLine("Outcome was True");
else
   Debug.WriteLine("Outcome was False");

That is all.

1 comment:

Rob The Geek said...

I too have worked with a colleague that didn't understand/know about the short-circuit operators.. Which scares me since C# was apparantly his primary language.

Anywho.

I am with you - IMO, all logical operators shout be short-circuit by default (or as a dev, we default to them). I almost mentally view it similar to the "fail fast" philosophy. As soon we know we are "not supposed to be here" - get the hell out of my face!

IF we have code that has to be executed on both sides of the operator (which in my experience is pretty rare) then I think the results should be assigned to variables, and then the variables evaluated (as you have done).

This makes what the code is actually doing a hell of a lot clearer.