Blog

Why C#’s “var” keyword can hamper maintainability (The Continuous Improver)

On January 29, 2016, in Syndicated, by Association for Software Testing
0

Coding conventions never cease to be a great source for heated debates. However, within the C# realm, two specific topics tend to reappear occasionally. The first one is about whether or not to use underscores for class fields (but I’m not going to discuss that here). The other one is the usage of the var keyword. I don’t know why, but during code reviews, the over-zealous usage of var always triggers a feeling of annoyance in me. So beware, this is going to be a very opinionated view on that.

In short, in my opinion (and those of my coding guidelines) var should only be used if the actual type is immediately visible from the statement you’re looking at. The only exception I can think of are anonymous types (often seen when using LINQ). So as far as I’m concerned, let’s check out some proper usage of var.

var largeOrders =
   from order in orders
   where order.Items > 10 and order.TotalValue > 1000
   select new { OrderId = order.Id };

var repository = new unitOfWork.GetRepository<Company>();       

var orders = new List<Order>();

All three examples should make it very clear what the involved type is, even if it is an anonymous type. Now for some dubious usage of var:

// I assume it’s a string, but maybe it’s some kind of domain-specific value type
var key = CommandNameResolver.GetName<IncludeLogbookEntryReferenceInHandoverCommand>();

// What was that default type C# used for numbers?
var i =3;

// No clue. An enumerable of DateTime maybe? And if so, why not use a TimeSpan?
var startOfShifts = GetShiftStartTimes();

// What codes? Strings? Guids? Something else?
foreach (var code in codes) { }

I know that modern IDEs like Visual Studio will make it easy to detect the type you’re looking at, but we do most code reviews – if not all – through GitHub Pull Requests. So consider this example screenshot.

clip_image001[4]

When I review a Pull Request that uses a var and the actual type is not immediately visible, I start to wonder about a couple of things.

  • If the var is assigned from a call to another method, how much information is that method returning. For instance, I wonder if it might return an entire class, even though the caller only needs a single value. A nice analogue is saying that you’re returning the entire freight truck even though you only need a single parcel. Especially if the call site is passing the return value to another method, it becomes important to understand what is being passed around.
  • If the variable name implies some kind of collection, might it be returning an IEnumerable<T>? If so, that may imply that its execution could be deferred. So what impact will that happen on the calling code? Is that second iteration I just noticed going to cause some weird side-effects?
  • If the statement implies a boolean outcome, will it return a nullable boolean? And if so, did the caller deal with that correctly.
  • Does the type being used originate from a project, component or NuGet package that isn’t supposed to be used in that part of the system?

I really can’t answer those questions without drilling a bit deeper in the code base, which is less than trivial on Github. Worst case I might have to pull down the sources into my local repo and use Visual Studio to understand the code being changed.

It’s widely accepted that source code is read many more times than it’s written. Now imagine you have to design an architecture for a system with similar characteristics. Wouldn’t you optimize your system for reading? If so, why wouldn’t you optimize your code for readability as well? Using a var just because you’re too lazy to type the full type name sounds like a lame excuse. And if you really that lazy, install Resharper and use ALT-ENTER to quickly switch the var with the explicit type. And the argument that using var will make it more easy to refactor your code is IMHO bull. If you change the type a method returns, don’t you want to make sure call sites are actually prepared for that rather than relying on compile time errors? And if it is, just use the infamous ALT-ENTER again…

All in all, I remain convinced that using an explicit type will help others understand your code much faster and will increase the change coupling errors will surface during code reviews. So what do you think? Let me know by commenting below. Oh, and follow me at @ddoomen to get regular updates on my everlasting quest for better solutions.

 

Comments are closed.


Looking for something?

Use the form below to search the site:


Still not finding what you're looking for? Drop a comment on a post or contact us so we can take care of it!