Recently we got a new contributor on RSS Bandit who uses the ReSharper refactoring tool. This has led to a number of changes to our code base due to suggestions from ReSharper. For the most part, I've considered these changes to be benign until recently. A few weeks ago, I grabbed a recent snap shot of the code from SVN and was surprised to see all the type declarations in method bodies replaced by implicitly typed variable declarations (aka the var keyword). Below is an excerpt of what some of the "refactored" code now look likes.

var selectedIsChild = NodeIsChildOf(TreeSelectedFeedsNode, selectedNode);
var isSmartOrAggregated = (selectedNode.Type == FeedNodeType.Finder ||
                                      selectedNode.Type == FeedNodeType.SmartFolder);
//mark all viewed stories as read 
if (listFeedItems.Items.Count > 0){
   listFeedItems.BeginUpdate();

   for (var i = 0; i < listFeedItems.Items.Count; i++){
       var lvi = listFeedItems.Items[i];
       var newsItem = (INewsItem) lvi.Key;

I found this change to be somewhat puzzling since while it may have shortened the code by a couple of characters on each line but at the cost of making the code less readable. For example, I can't tell what the type of the variable named lvi is just by looking at the code.

I did some searching online to see how the ReSharper team justified this refactoring "suggestion" and came across the blog post entitled  ReSharper vs C# 3.0 - Implicitly Typed Locals by a member of the ReSharper team which contains the following excerpt

Some cases where it seems just fine to suggest var are:

  1. New object creation expression: var dictionary = new Dictionary<int, string>();
  2. Cast expression: var element = (IElement)obj;
  3. Safe Cast expression: var element = obj as IElement;
  4. Generic method call with explicit type arguments, when return type is generic: var manager = serviceProvider.GetService<IManager>()
  5. Generic static method call or property with explicit type arguments, when return type is generic: var manager = Singleton<Manager>.Instance;

However, various code styles may need to suggest in other cases. For example, programming in functional style with small methods can benefit from suggesting every suitable local variable to be converted to var style. Or may be your project has IElement root interface and you just know that every variable with "element" name is IElement and you don't want explicit types for this case. Probably, any method with the name GetTreeNode() always return ITreeNode and you want vars for all such local variable.

Currently, we have two suggestions: one that suggests every suitable explicitly typed local to be converted to implicitly typed var, and another that suggests according to rules above.

So it seems there are two suggestion modes. The first suggests using the var keyword when the name of the type is obvious from the right hand side expression being evaluated such as casts or new object creation. The second mode suggests replacing type declarations with the var keyword anywhere the compiler can infer the type [which is pretty much everywhere except for a few edge cases such as when you want to initialize a variable with null at declaration]. 

The first suggestion mode makes sense to me since the code doesn't lose any clarity and it makes for shorter code. The second mode is the one I find problematic it takes information out of the equation to save a couple of characters per line of code. Each time someone is reading the code, they need to resort to using Go To Definition or Auto-Complete features of their IDE to tell something as simple as "what is the type of this object".

Unfortunately, the ReSharper developers seem to have dug in their heels religiously on this topic as can be seen in the post entitled Varification -- Using Implicitly Typed Locals where a number of arguments are made justifying always using implicitly typed variables including

  • It induces better naming for local variables.
  • It induces better API.
  • It induces variable initialization.
  • It removes code noise
  • It doesn't require using directive.

It's interesting how not only are almost all of these "benefits" mainly stylistic but how they contradict each other. For example, the claim that it leads to "better naming for local variables" really means it compels developers to use LONGER HUNGARIAN STYLE VARIABLE NAMES. Funny enough, these long variable names add more noise to the code overall since they show up everywhere the variable is used compared to a single type name showing up when the variable is declared. The argument that it leads to "better API" is another variation of this theme since it argues that if you are compelled to use LONGER MORE DESCRIPTIVE PROPERTY NAMES (e.g. XmlNode.XmlNodeName instead of XmlNode.Name) then this is an improvement.  Someone should inform the ReSharper folks that encoding type information in variable names sucks, that's why we're using a strongly typed programming language like C# in the first place.

One more thing, the claim that it encourages variable initialization is weird given that the C# compiler already enforces that. More importantly, the common scenario of initializing a variable to null before it is used isn't supported by the var keyword.

In conclusion, it seems to me that someone on the ReSharper team went overboard in wanting to support the new features in C# 3.0 in their product even though in some cases using these features cause more problems than they solve. Amusingly enough, the C# 3.0 language designers foresaw this problem and put the following note in the C# language reference

Remarks: Overuse of var can make source code less readable for others. It is recommended to use var only when it is necessary, that is, when the variable will be used to store an anonymous type or a collection of anonymous types.

Case closed.

Now Playing: Mariah Carey - Side Effects (featuring Young Jeezy)


 

Thursday, 22 May 2008 08:28:08 (GMT Daylight Time, UTC+01:00)
Amen! This one took me by surprise as well and is one of the first options I turned off (well, turned down).
Thursday, 22 May 2008 08:53:33 (GMT Daylight Time, UTC+01:00)
I posted a comment (on the R# website) in response to the R# team posts pretty much saying exactly what you did. For some reason it never made it out of moderation and into the comments. Good to see the moderation is working ?!

I always make an effort anyway to name things sensibly but the use of 'var' loses information. If I am looking at someone elses code to debug something I don't want to have to stop at every 'var' usage and work out what it is.
Mark Rainey
Thursday, 22 May 2008 14:41:15 (GMT Daylight Time, UTC+01:00)
It drives me nuts, each time I install the latest build it goes back to changing to var, so annoying.
I find my own code unreadable with vars, I use good naming but if you got a list you want to know what type it is, reminds me too much of vb6 Variant makes me feel dirty :)
Thursday, 22 May 2008 19:11:57 (GMT Daylight Time, UTC+01:00)
The case isn't closed. I find type declarations all over the place to make code "less readable". I just name my locals appropriately.
Mark Penillas
Thursday, 22 May 2008 19:30:29 (GMT Daylight Time, UTC+01:00)
I totally agree that var should only be used when it needs to be i.e. when the type the var represents is anonymous. C# is not a dynamically-typed language. It uses dynamic typing to accomplish some neat things. But it is still strongly-typed and we should all be using it as such.

If you want to program with dynamic types then use a truly dynamic language. Simple enough.
Saturday, 24 May 2008 09:07:19 (GMT Daylight Time, UTC+01:00)
[note: treat [<] below as a '<' char since your server doesn't like them - see note below]
-----------------------------

As with all things, moderation is the answer. It makes sense to use var for variable declarations when the right-hand-side clearly shows the type, as in the first 3 items in the "suggested use" list.

For me as a C -> C++ -> Java -> C# developer the most annoying aspect of the language is repeating the type in variable declaration-and-assignment statements. For example:

SomeReallyLongGenericTypeName[<]SomeEquallyLongTypeArgumentName> foo = new SomeReallyLongGenericTypeName[<]SomeEquallyLongTypeArgumentName>();

is just a waste of keystrokes. Even VB (ack!) has required only one mention of a variable's type in its declaration-and-assignment for the last couple (few?) of versions, so when I saw var added to C# 3.0 my joy was unbounded!

So, although var was obviously introduced to help with anonymous type handling, I believe it can also be used clearly and efficiently for variable declaration-and-assignment statements *with either a new or a cast on the right-hand-side* which makes the type clear.

Finally, use of var for assigning the return value of some obscure method call where it's not clear what the return type is, should be a hanging offence. Not that I'm at all religious about programming style, you understand...

P.S. Good to see you blogging again.


-----------------------------
Note: My comment included use of '<' and '>' in the generic type example, and your server barfed on it:

Server Error in '/weblog' Application.
A potentially dangerous Request.Form value was detected from the client (ctl01$comment="...icTypeName[<]SomeEquallyLongType...").
Philip the Duck
Saturday, 24 May 2008 11:49:19 (GMT Daylight Time, UTC+01:00)
As Philip the Duck says, it's a judgment call. As It's perfectly ok to use the var 'contextual token' (not keyword) when you're newing up something with a huge long name. That does help readability. But ReSharper *has* gone way OTT. Why would I want to change int foo = 1; to var foo = 1; ? I don't even save on letters.

Also, as of ReSharper 4.0 beta (released yesterday), in VS 2008, when targeting the 2.0 framwork it still suggests the var refactor.
Wednesday, 28 May 2008 10:48:40 (GMT Daylight Time, UTC+01:00)
>> var selectedIsChild = NodeIsChildOf(TreeSelectedFeedsNode, selectedNode);
>> var isSmartOrAggregated = (selectedNode.Type == FeedNodeType.Finder ||

That is not how resharper suggests it? atleast not with the config I have.

Resharper suggests the use of var when the type is explicit on the right hand side.
var foo = new Foo()

But not:

var foo = GetFoo();

Thursday, 29 May 2008 03:23:02 (GMT Daylight Time, UTC+01:00)
i've been thinking about the same issues when it came to code i am working on. common sense argues for you, furthermore, it is true what have you said. however, i've decided to go with r# suggestions and use var wherever is possible. thought that turned me into this direction has been composite: 1) it is my code and i know all about it, 2) i should get used to var and way of thinking it introduces for the future.
i've been learning f# past few months, so it is possible that my decision inferred from that fact.

best,
ace
Friday, 30 May 2008 21:18:49 (GMT Daylight Time, UTC+01:00)
vars are useful for variables used to store intermediate data. If you have a method that returns some data and other that consumesthese data, vars allow simple changes of type passed between methods.

While such things may be a sign of bad design, it often happens that you have multiple cases of similiar code.

var data = GetData();
ProcessData(data);

It is better than just ProcessData(GetData()) and still I am safe to change type returned by and consumed by these methods.

I just don't want to know what is returned and what is consumed.

Yuriy
Comments are closed.