Are these 2 statements identical?

Do the following 2 code snippets achieve the same thing?

My original code:

if (safeFileNames != null)
{
    this.SafeFileNames = Convert.ToBoolean(safeFileNames.Value);
}
else
{
    this.SafeFileNames = false;
}

What ReSharper thought was a better idea:

this.SafeFileNames = safeFileNames != null && 
                     Convert.ToBoolean(safeFileNames.Value);

I think the above code is much easier to read, any compelling reason to change it?

Would it execute faster, and most importantly, will the code do the exact same thing?

Also if you look at the : Convert.ToBoolean(safeFileNames.Value); section, then surely this could cause a null reference exception?

this.SafeFileNames = bool

Local safeFileNames is a strongly typed custom object, here is the class:

public class Configuration
    {
   开发者_开发问答     public string Name
        {
            get;
            set;
        }
        public string Value
        {
            get;
            set;
        }
    }

The fact that you asked the question suggests to me that the former is preferred. That is, it seems to me that your question implies that you believe the first code to be easier to understand, and you aren't quite sure if the second is equivalent. A primary goal of software design is to manage complexity. If it's confusing to you now, it may also be to you later, or to whomever supports your code down the road.

Both statements do the exact same thing. Which to use is a matter of preference, but I prefer Resharper's version. More concise, less moving parts. Easier to see the intent of the code.

Here's IL for both pieces of code. I took your code and created a console app to take a look at the IL. As you can see from the resulting IL, one method (method2) is 4 bytes shorter, but the IL that runs for both is pretty much the same, so as far as performance goes... don't worry about that. They both will have the same performance. Focus more on which one is easier to read and better demonstrates your intent.

My code:

class Program
{
    static void Main(string[] args)
    {


    }
    public void method1()
    {
        bool? safeFileNames = null;

        if (safeFileNames != null)
        {
            SafeFileNames = Convert.ToBoolean(safeFileNames.Value);
        }
        else
        {
            SafeFileNames = false;
        }
    }
    public void method2()
    {
        bool? safeFileNames = null;
        SafeFileNames = safeFileNames != null && Convert.ToBoolean(safeFileNames.Value);
    }
    public static bool SafeFileNames { get; set; }
}

The IL for method 1:

.method public hidebysig instance void  method1() cil managed
{
  // Code size       42 (0x2a)
  .maxstack  1
  .locals init ([0] valuetype [mscorlib]System.Nullable`1<bool> safeFileNames)
  IL_0000:  ldloca.s   safeFileNames
  IL_0002:  initobj    valuetype [mscorlib]System.Nullable`1<bool>
  IL_0008:  ldloca.s   safeFileNames
  IL_000a:  call       instance bool valuetype [mscorlib]System.Nullable`1<bool>::get_HasValue()
  IL_000f:  brfalse.s  IL_0023
  IL_0011:  ldloca.s   safeFileNames
  IL_0013:  call       instance !0 valuetype [mscorlib]System.Nullable`1<bool>::get_Value()
  IL_0018:  call       bool [mscorlib]System.Convert::ToBoolean(bool)
  IL_001d:  call       void ConsoleApplication5.Program::set_SafeFileNames(bool)
  IL_0022:  ret
  IL_0023:  ldc.i4.0
  IL_0024:  call       void ConsoleApplication5.Program::set_SafeFileNames(bool)
  IL_0029:  ret
} // end of method Program::method1

The IL for method2:

.method public hidebysig instance void  method2() cil managed
{
  // Code size       38 (0x26)
  .maxstack  1
  .locals init ([0] valuetype [mscorlib]System.Nullable`1<bool> safeFileNames)
  IL_0000:  ldloca.s   safeFileNames
  IL_0002:  initobj    valuetype [mscorlib]System.Nullable`1<bool>
  IL_0008:  ldloca.s   safeFileNames
  IL_000a:  call       instance bool valuetype [mscorlib]System.Nullable`1<bool>::get_HasValue()
  IL_000f:  brfalse.s  IL_001f
  IL_0011:  ldloca.s   safeFileNames
  IL_0013:  call       instance !0 valuetype [mscorlib]System.Nullable`1<bool>::get_Value()
  IL_0018:  call       bool [mscorlib]System.Convert::ToBoolean(bool)
  IL_001d:  br.s       IL_0020
  IL_001f:  ldc.i4.0
  IL_0020:  call       void ConsoleApplication5.Program::set_SafeFileNames(bool)
  IL_0025:  ret
} // end of method Program::method2

It reduces the cyclomatic complexity of your code by using the resharper suggestion.

Whether it's easier to read or not, is a personal opinion, however I prefer the suggestion resharper gave you.

They are identical, and if you want readability, I could also suggest the following:

if (safeFileNames != null)
    this.SafeFileNames = Convert.ToBoolean(safeFileNames.Value);
else
    this.SafeFileNames = false;

or

this.SafeFileNames = safeFileNames != null ? Convert.ToBoolean(safeFileNames.Value) : false

They're the same. The && is a short-circuiting operator so the second half of the expression won't evaluate if safeFileNames is null.

They are the same. In the one-liner, if the first condition fails, the second is not evaluated. So you don't get a null reference.

I bet the IL is the same in both cases.

I prefer the second version.

Yep both those statements will do the same thing, you don't have to take re-sharpers suggestions as gospel, what one man considers to be readable code is another mans mess.

There are a few other ways to do what your trying to do that might be more readable, what value type is safeFileNames? It looks like it may be a nullable bool? If so you could simply just write,

this.SafeFileNames = safeFileNames.GetValueOrDefault();

Logically, they're identical. Any performance difference would likely be neglible. It's possible that the second form would translate to more efficient binary code on some platforms since the second form does away with a conditional. Conditionals (incorrect speculative execution) can mess up your CPU's instruction pipeline in CPU-intensive work. However, both the IL and the JITter need to emit code of adequate quality for this to make much difference.

I agree with your sense of readability, but I don't assume everyone shares that.