Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I had the following two lines of code:

RegisteredForMeasurement = false;
return EngineCommands.CommandReset();

Now I need to change this so RegisteredForMeasurement is set to false AFTER the CommandReset. I have two possiblities I can think of:

var result = EngineCommands.CommandReset();
RegisteredForMeasurement = false;
return result;

or

try { return EngineCommands.CommandReset(); }
finally { RegisteredForMeasurement = false; }

Aside from the fact, that in version 2 RegisteredForMeasurement is also set to false if CommandReset throws an exeption, what would you prefer. Or is there an even better or more elegant way?

Is version 2 faster, as it does not need a temporary local variable or does it end up to be about the same after JIT-optimizations?

People always say you should not use try/catch for "program logic" - is this also true for try/finally?

I used the second version a few times in the past, when I had multiple returns and needed to execute something after all of them, as it "reads more natural" to me ("do FINALLY after everything else is done").

share|improve this question
5  
Just because you can use try/finally for your scenario doesn't mean you should. Try/finally has a purpose and carries semantic weight - AFAIK it has not been borrowed (a la using) for other reasons, so I'd strongly suggest doing it the least clever way. –  Mark Brackett May 23 at 19:57

3 Answers 3

up vote 4 down vote accepted

"Is version 2 faster, as it does not need a temporary local variable or does it end up to be about the same after JIT-optimizations?"

On the contrary. Even if using try..finally has litte overhead, a local variable is very, very cheap.

The extra time for allocating a local variable is actually zero. The space for the local variable is allocated by moving the stack pointer, and that is done anyway when the stack frame is created for the method, so the allocation is completely free.

Storing the value in the variable is cheaper than the extra work needed to set up the exception handling.

Just for fun I benchmarked the two methods, and using try..finally takes about 50% longer in x64 mode and about 200% longer in x86 mode. However, to get some perspective consider that the time difference is about a nanosecond.

share|improve this answer

There's a very big difference between the two options you have presented and they don't fall into the categories of readability, elegance etc. It falls into realm of correctness.

Code in a finally block always executes even if an exception is thrown in the try block. So, if it's critical to the program's execution that RegisteredForMeasurement needs to be assigned to false, even in the event of an exception, then you would you would do that in the finally block. If it's critical that it isn't assigned in the event of an exception, then you would code accordingly.

When we talk about readability or elegance, we're talking about code that does exactly the same thing, but designed in a way that is easier to comprehend. In this case, it doesn't apply because you're talking about two pieces of code that perform very different things in different scenarios.

share|improve this answer
    
I am well aware of this and also wrote that in my question, but in my case it makes no difference, as if there is an exception it is handled "globally" and so it does not matter what RegisteredForMeasurement is in that case. Because of this, the functional difference of the two methods do not matter to me in this case, why it comes down to readability or elegance for me... –  ChrFin May 23 at 13:24
5  
In this case then less is more; if it doesn't matter, then use less code. Just be aware that when some reads try/finally, they're going to assume that the code in the finally block is absolutely critical to the working of the program and not because its readable or elegant. –  George Howarth May 23 at 13:32

Using try/finally is perfectly alright to use for programming logic. The reason you shouldn't use try/catch for logic is because it breaks up program flow, and can be time consuming(especially in debug mode). However, using the finally portion of a try just to return a value later is ayokay. I would caution you though to keep your code readable, and readily understandable, and a try-finally may hinder you in that endeavor.

Though to make your own educated decision I also want to show you the difference it makes in the IL (which C# is compiled down to). For the purposes of this example I assumed that whatever was being returned by EngineCommands.CommandReset() was just an object. For each of these I've showed the C# then its compiled IL.


var result = EngineCommands.CommandReset();
RegisteredForMeasurement = false;
return result;

Results in the following:

.maxstack 1
.locals init ( [0] object result )

IL_0000: call object EngineCommands::CommandReset()
IL_0005: stloc.0
IL_0006: ldc.i4.0
IL_0007: call void Program::set_RegisteredForMeasurement(bool)
IL_000c: ldloc.0
IL_000d: ret

try { return EngineCommands.CommandReset(); }
finally { RegisteredForMeasurement = false; }

Results in the following:

.maxstack 1
.locals init ( [0] object CS$1$0000 )

.try
{
    IL_0000: call object EngineCommands::CommandReset()
    IL_0005: stloc.0
    IL_0006: leave.s IL_000f
}
finally
{
    IL_0008: ldc.i4.0
    IL_0009: call void Program::set_RegisteredForMeasurement(bool)
    IL_000e: endfinally
}

IL_000f: ldloc.0
IL_0010: ret
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.