2

I have method a the returns something like this:

List<Customer> customers = repository.GetCustomers.Where(x => x.IsActive);

return customers;

Visual Studio tooling is suggesting an inline temporary variable:

return repository.GetCustomers.Where(x => x.IsActive);

Years ago I heard a tip on a .Net podcast (the guest I don't recall) that there are some memory/performance advantages to the first option.

The micro-optimization question talks about this issue in a broad context. But is it indeed micro-optimization or style preference only?

Mike Henderson
  • 239

3 Answers3

5

Having a local variable is good for debugging, but I would remove it when it is no longer needed for the sake of readability.

There is indeed a difference in generated IL and performance between the two, but it is infinitesimally small compared to the data-bound calls in your application. I had to iterate billions of times before seeing any timing difference.

Bottom line, it doesn't matter. Visual Studio (and by extension, Microsoft) only suggests that you remove the local variable to improve readability.

This is a style preference.

Dan Wilson
  • 3,103
4

Assuming the return type is also List<Customer> I would find it surprising if there were any difference in the resulting code between the two options. I would prefer the first one because it is more debug-friendly. It allows you to stop on the return statement and inspect the collection.

Martin Maat
  • 18,435
  • I like the first option too for the same reason. – Mike Henderson Sep 15 '18 at 15:44
  • possibly worth checking to see if the same IL is indeed generated – Ewan Sep 15 '18 at 16:37
  • The 1st option also allows you to better document the function by giving the temp variable a good name. This should not be necessary, as, ideally, the function already has a good name. But, if the function has a poor or very generic name, say, to match an old API, naming the temp variable can add value. In this example, name it activeCustomers. Plus, it is nice for debugging. – user949300 Sep 15 '18 at 19:05
  • @Ewan Yeah, someone should do that. :-P – Martin Maat Sep 15 '18 at 19:20
4

Since for both other answers the main advantage mentioned is 'for debugging' here's a tip: the debugger displays return values of functions, even nested ones, in both the Autos and Locals windows these days (accessible via Debug->Windows menu).

So if you put a breakpoint on the line return repository.GetCustomers.Where(x => x.IsActive); then do a single step (F10 usually), you'll get an entry with Name System.Linq.Enumerable.Where<TSource> returned in the watch windows where you can inspect the Value and Type just like any other variable. And in this case you'll also get an entry for what GetCustomers returned. Here's a screenshot for clarity:

enter image description here

tldr; if you've got the tooling there's really no need for using a variable. The alternative is clearer, shorter, can be used as Expression-bodied member.

The only other thing I can think of is that somehow want to anticipate that you're going to need the variable later and want to minimize the version control diff, but that falls under YAGNI so I wouldn't personally do that.

stijn
  • 4,138