Stop the style bashing

I might have mentioned before that I try to spend some time every day answering questions on StackOverflow. I learn a lot about the common issues that other programmers face, and I’ve picked up quite a few tips from reading answers supplied by others. I also learn quite a bit when I answer questions; sometimes providing a complete useful answer requires a lot of research.

Note that the discussion below is specific to C#. I’m not familiar with the implementation of the bool data type in C++ or boolean in Java, and C didn’t have a separate Boolean type.

I also learn about others’ misconceptions. For example, whenever somebody posts code that compares a Boolean value to true or false, at least one person will say something like “Never write == true or == false.” One person said, “Whenever you write == true or == false, God kills a kitten.”.

Those types of comments are pretty annoying. It’s just a style thing. We all know that these two conditionals are equivalent:

    if (timer1.Enabled == true) { ... }
    if (timer1.Enabled) { ... }  // the "== true" is implied

Complaining about the code containing == true is like complaining about using braces to enclose a single statement, or the extraneous parentheses in x = x + (y/z);, or getting mad because the person didn’t write x += y/z;. It’s a style thing! If you don’t like it, just move on.

Until recently, I honestly thought that’s all it was: immature griping about a minor style issue. But recent comments indicate that several people think there’s some difference in the generated code. That is, they think that if (timer1.Enabled == true) requires an extra operation to compare the values! That’s not true at all, as you can clearly demonstrate by compiling a simple program and looking at the generated code.

    System.Timers.Timer myTimer = new Timer();
    if (timer1.Enabled) Console.WriteLine("Enabled!");
    if (timer1.Enabled == true) Console.WriteLine("Enabled");

Compiling that in Release mode with Visual Studio 2013 results in the following intermediate code. I’ve added a few comments.

  // start of first conditional: if (timer1.Enabled) ...
  IL_0000:  ldarg.0
  IL_0001:  ldfld      class [System]System.Timers.Timer sotesto.Program::myTimer
  IL_0006:  callvirt   instance bool [System]System.Timers.Timer::get_Enabled()
  IL_000b:  brfalse.s  IL_0017
  IL_000d:  ldstr      "Enabled!"
  IL_0012:  call       void [mscorlib]System.Console::WriteLine(string)
  // start of second conditional if (timer1.Enabled == true)
  IL_0017:  ldarg.0
  IL_0018:  ldfld      class [System]System.Timers.Timer sotesto.Program::myTimer
  IL_001d:  callvirt   instance bool [System]System.Timers.Timer::get_Enabled()
  IL_0022:  brfalse.s  IL_002e
  IL_0024:  ldstr      "Enabled!"
  IL_0029:  call       void [mscorlib]System.Console::WriteLine(string)

Those two bits of code are identical. Each one calls the get accessor for the myTimer.Enabled property, and then branches around the WriteLine if the value is false.

Note that this is intermediate code. The JIT compiler might be able to inline the get_Elapsed method, which would eliminate the method call. Regardless, the code for these two snippets is identical. So stop with the == true bashing already.