8

Which way is better practice: return a value from a method inside an using statement or declare a variable before, set it inside and return it after?

public int Foo()
{
  using(..)
  {
     return bar;
  }
}

or

public int Foo()
{
  var b = null;
  using(..)
  {
    b = bar;
  }
  return b;
}
Jon Seigel
  • 12,251
  • 8
  • 58
  • 92
abatishchev
  • 98,240
  • 88
  • 296
  • 433

7 Answers7

9

I prefer the first example. Fewer variables, fewer lines of code, easier to follow, easier to maintain...

public int Foo()
{
  using(..)
  {
     return bar;
  }
}
Scott Ivey
  • 40,768
  • 21
  • 80
  • 118
5

Following the "less is more" principle (really just a variant of KISS), the former. There are fewer lines of code to maintain, no change in semantics and no loss of readability (arguably this style is easier to read).

jason
  • 236,483
  • 35
  • 423
  • 525
5

From using Statement - MSDN

The using statement ensures that Dispose is called even if an exception occurs while you are calling methods on the object. You can achieve the same result by putting the object inside a try block and then calling Dispose in a finally block; in fact, this is how the using statement is translated by the compiler.

From the try-finally (C# Reference)

finally is used to guarantee a statement block of code executes regardless of how the preceding try block is exited.

To answer your question, yes its okay to return from a using statement.

Stan R.
  • 15,757
  • 4
  • 50
  • 58
  • He asked which of the above is better practice, not for the semantics of the using statement. Arguably the semantics are relevant here, but it doesn't answer the question posed. – jason Aug 03 '09 at 18:34
  • @Jason: fair enough..the title of the question is "Is it OK to return from a method inside a using statement?" .. you could see i am not the only one who read the question this way. – Stan R. Aug 03 '09 at 18:36
  • 1
    @Stan R: I see the potential for confusion, but the post clearly is asking about best practices, not whether or not the finally block will execute (i.e., semantics). – jason Aug 03 '09 at 18:38
3

The second is clearly better, and you can verify that it works fine by writing a test program.

The using statement itself can't have a value, which is a limitation. Suppose you have a method called Open that returns an open FileStream, and you want to get the length of the file:

Console.WriteLine(Open().Length);

The bug there is that you aren't disposing the FileStream. So you have to write (similar to your example):

long length;

using (FileStream file = Open())
    length = file.Length;

Console.WriteLine(length);

But with a simple extension method, you can write this instead:

Console.WriteLine(Open().Use(file => file.Length));

Nice and neat, and the FileStream gets properly disposed of.

Daniel Earwicker
  • 114,894
  • 38
  • 205
  • 284
  • Neat, but unreadable at the same time :-) – Scott P Aug 03 '09 at 18:41
  • heh this is pretty neat. thx for a clever idea. – Stan R. Aug 03 '09 at 18:41
  • @Scott P - it's essentially the same idea as the `? :` operator (an "expression" equivalent to `if`). Some find `? :` less readable than the equivalent `if` statement, I find it can be clearer sometimes. Eliminating variable names often helps with clarity. – Daniel Earwicker Aug 03 '09 at 18:45
2

No reason not to since that using statement translates into a try...finally block and the finally part is guaranteed to be executed (even through a return or an unhandled exception).

Blindy
  • 65,249
  • 10
  • 91
  • 131
  • 1
    Someone always has to point this out - the finally block is *not* "guaranteed" to be executed at all. There are well-defined circumstances under which it will execute, and others where it won't. – Daniel Earwicker Aug 03 '09 at 18:30
  • 1
    Have some examples where it won't? – Jordan Parmer Aug 03 '09 at 18:31
  • 1
    @Earwicker: if the cases where it won't get executed amount to < 0.01%, then I think for most purposes, we can say it's "guaranteed". – John Saunders Aug 03 '09 at 18:31
  • Looking at Stan R's quote from the C# Reference, it would appear that you are wrong about that. – Blindy Aug 03 '09 at 18:32
  • 1
    It's guaranteed to run in the same sense any other code is guaranteed to run. – Joel Coehoorn Aug 03 '09 at 18:33
  • @Joel Coelhoorn - which is to say, it's not guaranteed to run. – Daniel Earwicker Aug 03 '09 at 18:39
  • @j0rd4n - many people find this page a handy reference: http://thedailywtf.com/Articles/My-Tales.aspx – Daniel Earwicker Aug 03 '09 at 18:40
  • Not that I agree much with the conclusion of it - `finally` blocks are a better place to put critically important transaction code than anywhere else. It's just very important to know how to stop them running when it's better that they don't run (e.g. an unexpected invalid program state). – Daniel Earwicker Aug 03 '09 at 18:43
  • @Earwicker: I could always turn the computer off, in which case, it won't run. – John Saunders Aug 03 '09 at 18:44
  • @Earwicker: I bet 99.9% of all .NET developers could go their entire career without having to think about how to stop a finally block from executing. It only adds confusion to bring such edge cases into a discussion like this. – John Saunders Aug 03 '09 at 18:45
  • @John Saunders - I think we can all benefit from ensuring that when our program crashes it doesn't go continue to try and "clean up" and (for example) delete a bunch of temporary files, and potentially delete the wrong ones, or destroy the evidence we need to identify the problem, or throw more exceptions (obscuring the original one). – Daniel Earwicker Aug 03 '09 at 18:50
1

It really comes down to personal preference. You'll find arguments on both sides of this particular fence. Myself, I favor option 1: returning as soon as you can. I believe it expresses the intent of the code better; there's no reason to stick around longer than you have to. If you've completed all your work, return.

Sometimes, you'll have multiple possible return points, and "end-of-method" work (logging, cleanup) that might lead you to a single return statement. Nothing terrible about that, but you can often handle those situations in finally blocks or with aspects in aspect-oriented programming.

Michael Petrotta
  • 59,888
  • 27
  • 145
  • 179
  • 1
    I agree about the personal preference. I prefer to do returning at the bottom/end of the method. Coding guidelines often dictate this as well. This probably also comes from the fact that methods are sometimes way too long, which results in unreadable methods. Keep them really short and your methods are readable again. So this is one of those "it depends" answers I guess! ;) Looking at them from best practices, afaik there really isn't one, it's more a matter of preference. – Dennis van der Stelt Aug 03 '09 at 18:55
1

I feel second one better 

public int Foo()
{
  using(..)
  {
     return bar;
  }
}

One thing that arises in mind while using this way is that we are returning in between the using so will the object(that we have wrapped in using ) will get disposed ,the Answer is yes because A using statement is just the mixture of try/finally block, it's fine to return from a try block too.The return expression will be evaluated, then the finally block will be executed, and the the method will return then.So go Ahead:)

Raghav
  • 8,772
  • 6
  • 82
  • 106