14

Is either one of these risky? Is one better? Or is it one of those things you print out and throw a dart at to decide?

I want to do this now that I understand how finally works:

try { 
    stuff that changes something... 
}
catch (System.Exception ex) { 
    something.worked = false; 
    something.err = ex.Message; 
}
finally { 
    stuff.close();
    return something; 
}

But I've seen:

try { 
    stuff that changes something...
    return something; 
}
catch (System.Exception ex) { 
    something.worked = false; 
    something.err = ex.Message; 
    return something; 
}
finally { 
    stuff.close(); 
}
ChuckNeuros
  • 151
  • 1
  • 1
  • 5
  • 2
    @Dlev - Except the first isn't valid C# so it cannot be done. So how can it be preferable? This question really should be asked again, one where any code is actually valid code, otherwise its sort of pointless. – Security Hound Aug 26 '11 at 16:27
  • @Ramhound Oops, I meant that the former isn't legal C#. The latter is actually valid syntactically. – dlev Aug 26 '11 at 16:29

3 Answers3

19

You can't return from finally. You will get compiler error:

Control cannot leave the body of a finally clause

If target class implements IDisposable then I would do next:

using (stuff s = new stuff())
{
    return stuff;
}

or

using (stuff s = new stuff())
{
    try
    {
        // do stuff
        return stuff;
    }
    catch (Exception ex)
    {
        // do logging or another stuff
        return something;
    }
}

will call Dispose() for you if that will be required/possible.

abatishchev
  • 98,240
  • 88
  • 296
  • 433
  • Doh! Try compiling before you ask next time, Self! It makes sense that control can't leave a finally since finally is so unique in demanding attention. – ChuckNeuros Aug 26 '11 at 16:01
  • 2
    @user540903: Well, yes, you should have compiled it. But don't beat yourself up too badly; there are languages where return from finally is legal, and those languages have some pretty strange control flow semantics as a result. It is a reasonable question in general, just not for C#. – Eric Lippert Aug 26 '11 at 16:33
  • @Eric I'm pretty Java is one of those languages. From what I recall, the most recent `return` overwrites any previous ones, so `try { throw new BlahException(); } catch { return 1; } finally { return 2; }` actually returns a `2` to the caller. – dlev Aug 26 '11 at 16:38
  • @dlev: Right. JavaScript also allows returns from finally with the same semantics; last return wins. – Eric Lippert Aug 26 '11 at 16:46
17

Personally I would do neither and would use


try { 
    stuff that changes something... 
}
catch (System.Exception ex) { 
    something.worked = false; 
    something.err = ex.Message; 
}
finally { 
    stuff.close();    
}
return something;

Also in the finally statement, check that you need to close/dispose of objects as they might have never been opened/set if they have failed.

Also see here Is it bad practice to return from within a try catch finally block?

Community
  • 1
  • 1
Tim B James
  • 20,084
  • 4
  • 73
  • 103
  • 1
    I agree with this, as abatishchev says you can't return in a finally, i don't think you should have a return in a catch. If you have a try catch and don't rethrow you want it return either way and if you rethrow then it will never return anyway. Either way you only need 1 return statement not 2 – Ben Robinson Aug 26 '11 at 15:57
  • 1
    returning from within the try/catch/finally is not considered "structured programming". I agree with Tim and Ben. – Bahri Gungor Aug 26 '11 at 16:16
0

There is no risk in the second approach. But it allows you to return different values in case of exceptions.

Stephan
  • 4,187
  • 1
  • 21
  • 33
  • 1
    So does returning a variable and setting it to different values in the try and catch blocks but only having 1 return outside the whole try/catach block. – Ben Robinson Aug 26 '11 at 16:00
  • returning the a value of a varible would also let you return different values in the case of exceptions. – Security Hound Aug 26 '11 at 16:26