Improve 1520, use of var

[niwrA]

• changed wording and argumentation from 'use only when' to 'use always unless'
• updated the link

[bkoelman]

This PR advocates to prefer var over explicit typing, which is not only a breaking change but also something I disagree with. So many times IDE support is unavailable, for example in PRs. The lack of types makes it harder to understand the code. And even when inside VS, it makes navigation harder. For example, when a method returns a list of customers, going to the underlying type for 'var' takes me to List<T> while I actually wanted to jump to the customer class (which becomes impossible).

[bkoelman]

What works well for me is to use var while writing code (which is faster), but then I reformat the file before commit. My formatting settings change var to explicit types unless the type is obvious. Resharper is very smart about this, understanding to use var on methods like CreateCustomer etc. This complies with the current rule and allows my reviewers to see types.

[niwrA]

This PR advocates to prefer var over explicit typing, which is not only a breaking change but also something I disagree with. So many times IDE support is unavailable, for example in PRs. The lack of types makes it harder to understand the code. And even when inside VS, it makes navigation harder. For example, when a method returns a list of customers, going to the underlying type for 'var' takes me to List<T> while I actually wanted to jump to the customer class (which becomes impossible).

If the lack of types makes it harder to understand the code, the semantic principle has been broken. If you want to argue that this principle is of lesser importance, then I know where our coding principles diverge to a point where we will likely never agree. But if otherwise, I would love to see you provide an example where it mattered and the semantic principle, the same one that Eric Lippens refers to, wasn't broken or significantly enhanced by explicit typing, and then of course not just the exception to the rule, but why it should be the guiding principle.

In your navigation example, navigate to the method definition and then jump to your type from there. It is not something I would sacrifice the semantic principle for.

Please, if you can, argue with code examples, and I will try to do the same.

[dennisdoomen]

I actually like the semantic aspect that @niwrA is proposing here and how this is such a big thing in DDD. I've been practicing the Ubiquitous Language for more than 10 years and think is crucial in capturing the business concepts in your code. So I propose the following compromise:

1. We rephrase AV1520 to something like "Only use var when the type is obvious or irrelevant to understand the context"
2. Introduce a new guideline to capture the semantic principle.

@bkoelman @niwrA what do you think?

[bkoelman]

I believe the guidance in this repo roughly falls into two categories: style (casing, spacing, line length, indents etc) and principles (single responsibility, dependencies, consistency etc). The first category is merely a matter of taste, where choosing something (and sticking with it) precedes over what is chosen and best automated using tools. The second category is more of a gray area, usually depends on context and where discussions result in new insights and deeper conceptual understandings.

'var' usage seems to fall in the first category, looking at Visual Studio formatting settings:

And Resharper formatting settings:

In my team we have configured style compliance as a PR validation step, making the build fail. I'm glad that we no longer have discussions on 'var', casing, line breaks or line length. At the same time, we use a reduced ruleset inside VS to avoid being distracted while writing code. A single key-press or command-line runner applies the configured styling. I'd hate to return to having discussion on 'var' again because we consider it contextual!

From time to time, proposals are done in this repo to change a recommended style, because there's always someone that is used to something else or just does not like the looks of it. The best advice to those would be to fork the repo and adapt to however they like it.

Let's not forget that C# is a general purpose language, where types are often essential to understanding the code. It is not a scripting language for a business rules engine that uses dynamic typing. For example, when I'm looking at code optimized for a hot path, I want to explicitly see usage of ReadOnlySpan<char> instead of string. When dealing with concurrency, I'd like to see usage of ConcurrentDictionary<,> or ImmutableDictionary<,> instead of IDictionary<,>. Types are often helpful, but it depends on what kind of code and the angle of the reader.

Using var everywhere makes people re-adopt practices from hungarian style. Compare:

Manager assignedTo = ...

vs:

var assignedToManager = ....

So if we were to reword this rule, I'd like it to better match existing tooling. Looks like we should use the word 'evident', whose exact meaning remains undefined and evolves over time as tooling gets smarter.

So I propose to change the title from:

Only use `var` when the type is very obvious (AV1520)

to:

Only use `var` when the type is evident (AV1520)

Aside from var usage, the discussion in the PR raises some other concerns:

• Prefer an interface over a class (covered by AV1004/AV1005)
• Emphasize purpose over mechanics (covered by AV1707/AV1712/AV1720)
• Use descriptive variable names (AV1706/AV1708)
• Prefer DSL entities over simple types (AV1140)
• Including units in variable names (would be a nice addition to AV1707)
• Did I miss any?

[niwrA]

@bkoelman I think that code analysis also defaults to suggesting using var. This is sometimes pretty annoying vs that Visual Studio defaults to explicit, but it seems that the trend is that both ReSharper and CodeAnalysis default to var.

My main issue with the whole implicit vs explicit thing, is that the wording of the rule here strongly implies that explicit is the default that can be deviated from only in very specific cases. This is not my experience - code that adheres to all the other principles will generally use var in 98% of all cases, is my experience. And in most PRs I see, explicit actively detracts from the readability of the code with superfluous long definitions, bad variable naming resulting from it, and hinders the refactorability.

Also, do you really believe that ImmutableDictionary<,> in the declaration of the variable that holds the result of the method is the place to verify that the code that returns the ImmutableDictionary<,> is the correct place to detect such errors in designing that method? I don’t think so.

@dennisdoomen I also want to point out that this PR review argument and being able to push PRs through is something that I consider a disease I want to stamp out. The result is that people review PRs and only look at superficial things like was string.Empty used instead of “”, explicit vs implicit, and not is the code readable and maintainable. Heck, most reviews should probably start with has this code been written with the proper tests and covered properly. We are checking for grammar, not understanding the sentence, and neglecting the purpose of the PR altogether.

[bkoelman]

@bkoelman I think that code analysis also defaults to suggesting using var. This is sometimes pretty annoying vs that Visual Studio defaults to explicit, but it seems that the trend is that both ReSharper and CodeAnalysis default to var.

Default configurations exist to not break existing users, not to advocate what's generally the best choice. So those defaults mean nothing to me. The fact they are configurable just indicates there's a general need for choice.

My main issue with the whole implicit vs explicit thing, is that the wording of the rule here strongly implies that explicit is the default that can be deviated from only in very specific cases. This is not my experience - code that adheres to all the other principles will generally use var in 98% of all cases, is my experience. And in most PRs I see, explicit actively detracts from the readability of the code with superfluous long definitions, bad variable naming resulting from it, and hinders the refactorability.

You've already made clear you'd like the wording reversed, which is what we disagree upon. If this needs to change in this repo, you'll need to convince @dennisdoomen on that, because he owns this repo. Pointing out unrelated problems like bad naming does make me switch to your side. Long names can be considered helpful or be considered annoying, depending on whether you value conciseness over being explicit. I value long descriptive names over abbreviations and meaningful names over including type info in the name. I've been using explicit types without hindering refactorability for years. Resharper existed long before 'var' was invented and has always provided refactoring support.

Also, do you really believe that ImmutableDictionary<,> in the declaration of the variable that holds the result of the method is the place to verify that the code that returns the ImmutableDictionary<,> is the correct place to detect such errors in designing that method? I don’t think so.

My point is that sometimes type info makes the difference. Sure there are ways to find the info I need by stepping into methods, loading up the IDE etc. But because they require additional effort to understand the code, they decrease readability.

But I think you're missing my point that var usage is a matter of style, similar to line breaks, spacing etc. So depending on what kind of code you write (simple/complex, business/technical, applications/services/libraries, functional/imperative/OO/procedural) and where you're coming from, you like one over the other, and that's okay.

[niwrA]

Visual studio’s editor is the odd one’s out and probably because of compatibility, but Resharper has changed this in the past, so it’s not just that, and Code Analysis is relatively new. And yes Resharper can do a lot of things for you, but it will still create a very cluttered pull request, if PR’s are so important ;). But that could be ok. It’s just moving problems around and saying yeah no refactoring is not a problem because some of us have tools for that.

A variable is repeated, throughout lines of code, whereas a declaration happens only once. So a good name is always more important to me, and more so than the declaration, because I should normally not have to go back to the declaration of a variable to see what type it is to understand what happens in the code.

You say it’s a matter of taste and I say it’s a matter of opinion of what constitutes good code. I don’t feel that is a matter of taste at all, but there are certainly different priorities in different contexts. However, I will still maintain that in by far the majority of contexts in C#, var is the better option.

I’m still looking forward to seeing specific examples, and especially would like to know if you guys think that considering the guidelines provided by Eric Lippens or even your own practices, var is in the minority.

I am genuinely curious - I want to always use the best possible approach and want to be able to explain why it is the best possible approach, and have data to back it up. There is an interesting research paper out there that did research across a large number of well known repositories, and could hardly detect a pattern or a preference in using explicit vs implicit. It’s chaos out there, and I’d like to see some order restored.

And I’m genuinely interested in knowing if my style of coding is so different and hard to understand versus what other people do, and if I’m missing an opportunity to further improve the readability of my code.

[dennisdoomen]

Use var when you have to; when you are using anonymous types. Use var when the type of the declaration is obvious from the initializer, especially if it is an object creation. This eliminates redundancy. Consider using var if the code emphasizes the semantic "business purpose" of the variable and downplays the "mechanical" details of its storage. Use explicit types if doing so is necessary for the code to be correctly understood and maintained. Use descriptive variable names regardless of whether you use "var". Variable names should represent the semantics of the variable, not details of its storage; "decimalRate" is bad; "interestRate" is good.

Given the discussions and many pros and cons, I'm going to run an experiment for a couple of weeks. I'm going to try to follow Eric's guidelines and see if it's causing any of loss of data.

[julianbartel]

I've been following this discussion and would like to put my 50 cents in.

Personally, I always prefer explicit typing over using var for the following reasons: Declaring a variable with var omits valuable information: a reading developer has to infer the type, thus, it puts an additional load on the developers mind while reading the code. While that's not a big deal for the experienced developer, it reduces productivity especially for less experienced developers or when you are outside of an IDE.

On the other hand, I don't see the benefit of using var. There are cases, where var is a must (anonymous types), and there are some very few cases, where var really improves readability (an example for those cases may be complex LINQ results or multiple nested generics in general). Are there other points beside readability? Maybe refactoring: Changing a return type of a method would not impact a declaration where that return value is assigned. But such a refactoring is likely to have a semantic impact which requires some changes anyway.

All in all: I feel that the pros of preferring var are not worth the trade-off in loosing readability.

Edit: As @dennisdoomen and @niwrA suggest, I will also now develop with this discussion in mind and see if there are situations where var fits better than explicit typing and vice versa.

[bkoelman]

Eric Lipperts guidance on var was useful at the time because we had nothing, but its manual aspect has become outdated since tooling took over and automated this. Making this a case-by-case decision, still today, would be fighting against common IDEs and their editorconfig support. To me this is more important than the actual config setting chosen. Anyone agrees or disagrees on that?

[niwrA]

@julianbartel wrote:

Declaring a variable with var omits valuable information: a reading developer has to infer the type, thus, it puts an additional load on the developers mind while reading the code.

This is the core of the discussion we are having though. I maintain, and Eric Lippens suggests as much, that the omitted information isn't valuable, and instead detracts from the information that is valuable:

Eric Lipperts wrote:

Consider using var if the code emphasizes the semantic "business purpose" of the variable and downplays the "mechanical" details of its storage.

I look forward to @dennisdoomen 's findings, and could recommend anyone to do that same exercise and share their results.

However, if you really think explicit should be the default, then you might as well remove the linked article, because I think it argues pretty strongly against it!

[bkoelman]

My team did not automate because it's cool, but because it was the only way to get adoption without negative atmosphere. I've played both roles below in the past :)

...And when that happens, the inevitable bickering starts, and you start to want to bang your head against the wall that I mentioned earlier. Arguments ensue. One team member appoints himself “standards cop” and starts hassling the others from a position of self-righteousness. Another team member adopts the passive-aggressive, “too cool to care” position. It can be a mess and a headache.

Unless, of course, you make such things impossible. There’s a great piece of advice that I’ve heard before, and it applies well to people management and new technique adoption both at the same time. Build a system, and, when things go wrong, blame the system.

https://blog.ndepend.com/developers-adopt-coding-standards/

[dennisdoomen]

No need to keep fighting. Again, I think the truth will be somewhere in the middle. As I don't believe in dogmatism, I'll try myself and see where I'll end up on the scale between implicit and explicit.

And if somebody really disagrees, fork the repo and build your own internal guidelines.

[niwrA]

No need to keep fighting. Again, I think the truth will be somewhere in the middle. As I don't believe in dogmatism, I'll try myself and see where I'll end up on the scale between implicit and explicit.

And? Any preliminary results?

And if somebody really disagrees, fork the repo and build your own internal guidelines.

Yes, of course, I had already done so. But this was basically my only disagreement with the excellent existing set, so that's why I wanted to discuss it, also to learn if maybe I missed something important.

[dennisdoomen]

And? Any preliminary results?

Didn't do enough coding yet.

[dennisdoomen]

After two years of experimenting, I've settled on the fact that there's no sensible default. Sometimes var is fine and sometimes you loose information. But if we want to rely on R# or Rider to assist on this, @bkoelman has shown me that sticking with the "Explicit type unless the type is evident" guideline is the best course of action.