4

We had a discussion at work about code design and one of the issues was when handling responses from a call to a boolean method like this:

bool ok = IsEverythingOK();

if(ok)
{
   //do somehthing
}

One of my colleagues insists that we skip the extra variable ok and write

if(IsEverythingOK())
{
   //do somehthing
}

Since he says that using the "bool ok"-statement is memorywise bad.

Which one is the one we should use?

Joakim M
  • 1,793
  • 2
  • 14
  • 29
  • 1
    You shouldn't worry about it. It's negligable. – Benjamin Diele Nov 10 '14 at 06:30
  • 1
    There is no difference in efficiency. I personally prefer the second form because I think it reads easier, but that's just my opinion. I would probably rename the function to EverythingIsOK, though. – 500 - Internal Server Error Nov 10 '14 at 06:32
  • 1
    @500-InternalServerError I find it strange you'd name a function (that returns a boolean) like that. Everywhere I've worked uses `IsSomethingValid` or a variant of that. – Benjamin Diele Nov 10 '14 at 06:34
  • 2
    IF you ever use the debugger, hovering over the 'ok' variable is also really nice :) – marko Nov 10 '14 at 06:35
  • I find the second one more readable, but memorywise it's really the same. At least if you aren't making a program for a toaster – Fabio Marcolini Nov 10 '14 at 06:44
  • 1
    Sure, because a `bool` uses 32 bits of memory it would be vastly better not to create a local variable. After all on a standard 8GB machine that would represent a waste of one 268,435,456th of the available memory. Whatever you do don't have a quarter of a million nested ifs! – Enigmativity Nov 10 '14 at 06:46

4 Answers4

5

Paraphrasing your question:

Is there a cost to using a local variable?

Because C# and .NET is well-engineered my expectation is that using a local variable as you describe has no or a negligible cost but let my try to back this expectation by some facts.

The following C# code

if (IsEverythingOk()) {
  ...
}

will compile into this (simplified) IL (with optimizations turned on)

call        IsEverythingOk
brfalse.s   AfterIfBody
... if body

Using a local variable

var ok = IsEverythingOk();
if (ok) {
  ...
}

you get this optimized (and simplified) IL:

call        IsEverythingOk
stloc.0
ldloc.0
brfalse.s   AfterIfBody
... if body

On the surface this seems slightly less efficient because the return value is stored on the stack and then retrived but the JIT compiler will also perform some optimizations.

You can see the actual machine code generated by debugging your application with native code debugging enabled. You have to do this using the release build and you also have to turn off the debugger option that supresses JIT optimizations on module load. Now you can put breakpoints in the code you want to inspect and then view the disassembly. Note, that the JIT is like a black box and the behavior I see on my computer may differ from what other people see on their computers. With that disclaimer in mind the assembly code I get for both versions of the code is (with a slight difference in how the call is performed):

call        IsEverythingOk
test        eax,eax  
je          AfterIfBody

So the JIT will optimize the extra unnecessary IL away. Actually, in my initial experiments the method IsEverythingOk returned true and the JIT was able to completely optimize the branch away. When I then switched to returning a field in the method the JIT would inline the call and access the field directly.

The bottom line: You should expect the JIT to optimize at least simple things like transient local variables even if the code generates some extra IL that seems unnecessary.

Martin Liversage
  • 104,481
  • 22
  • 209
  • 256
3

It all depends on whether you are doing anything with ok inside the loop.

i.e.

bool ok = IsEverythingOK();

if(ok)
{
   //do somehthing
   ok = IsEverythingOK();
}

Assuming you don't do anything with ok in the loop however, you will probably find that the JIT compiler will turn:

bool ok = IsEverythingOK();

if(ok)
{
   //do somehthing
}

...into what is essentially:

if(IsEverythingOK())
{
   //do somehthing
}

...anyway.

  • 1
    I'm sorry but it is not right. As you can see in disassambler the compiler will not optimize this code although `ok` is not used any more. Using `ok` in fact generates a little bit more code. – Fratyx Nov 10 '14 at 07:01
1

The compiler of course generates some additional IL code steps if you use the first solution as it at least needs an additional stloc and a ldloc command but if it's just for performance reasons, forget these microsecondes (or nanoseconds).

If there is no other reason for the ok variable I would prefer the second solution nevertheless as it is easier readable.

Fratyx
  • 5,717
  • 1
  • 12
  • 22
1

I'd say it's preference, unless you'd have a unified coding standard. One of the other provides a benefit.

This is great if you're expecting or assuming modification other than the if clause. Though it does create a stack entry upon creation of the variable, it'll prolly get disposed after the method scope.

bool ok = IsEverythingOK();

if(ok)
{
   //do somehthing
}

This is great if you only want to use it as a validation one. Though it's only good if your method name is short. But lets say you access a class before you use it like _myLongNameInstance.IsEverythingOK() this reduces readability and I'll go with the first one, but with other conditions, i'd choose the direct if.

if(IsEverythingOK())
{
   //do somehthing
}
DevEstacion
  • 1,947
  • 1
  • 14
  • 28