Using 'this' incorrectly in C#
jeremyaboyd.com
jeremyaboyd.com
Because it's consistent with the pattern of [object].[method](). When you call a method in class B from class A, the call is b.someMethod(). When you call a method in class A from another method in class A, the call is this.someMethod().
I personally find that the consistency makes the code easier to parse and read. If I join an existing project where "this" isn't being used already, I wouldn't push it. As long as your consistent either way it's not a big deal, but for new projects where I get a say in it, it is my preference.
I generally agree with "consistency is good", but for an issue like this: honestly, who cares? This is SUCH a small difference that even if you're inconsistent, I doubt I'd even notice it or pay it a second thought. If you're going to work with other human beings, you can't be this insanely dogmatic, you'll never get anything done.
So, if you've got Go code like this:
type B struct {
int c
}
type A struct {
B
}
And, as with inheritance, members+methods defined explicitly on the parent (A) shadow the ones from the child (B), when you're accessing them on the parent. So you can do something that looks a lot like "subclass method overriding" on the child. (Though it isn't, quite, because the embedded child's methods, once reached, will only call each-other, oblivious to the parent. In C++ terms, there are no "virtual" methods.)
This is more often used for decoration or aggregation (e.g. Go's bufio.ReadWriter, which is just a struct embedding a bufio.Reader and a bufio.Writer), but it can be used to simulate inheritance pretty well, with almost none of the disadvantages that come from having inheritance built into your type system.
(For example, there's no need for the Liskov Substitution Principle in Golang, since you can't pass an embedding "subclass" (A) instance into a method parameter that wants the "base class" type (B). If you want that behavior, you opt into it explicitly by defining the method parameter as being of an interface type, where the interface is one that B implements; then, anything that embeds B will—unless it shadowed B's methods with methods of different signatures—also meet that interface.)
> decorate your own method
> I personally find that the consistency makes the code easier to parse and read.
> As long as your consistent
knowing when to break consistency is a sign of education and superiority. The point of it is to be difficult, to make those who know identifiable and part of a club, whether the club is grammar nazis or, well, grammar nazis.
If only we could make a push to regularize and consistentize more things, without it being called "dumbing down" or "satire".
Yes, there is a reason the saying "a foolish consistency is the hobgoblin of little minds" is a thing. (Incidentally, it's a quote from Ralph Waldo Emerson.) But, my experience, and that of many others is that having a consistently styled codebase makes things a lot easier when making changes later.
Yes, there are tradeoffs, but, in engineering, we learn to figure out which tradeoffs are worth making. That's the point people are trying to make, and it is not a trivial one.
Edit: also note the word "foolish" in this context. "Foolish consistency" would be taking the consistency principle to levels where it no longer provides benefits.
Why are you saying it in the style of disagreeing?
> Complaining about downvotes.
I'm not complaining about downvotes, I'm accusing people of not reading. I think people saw my comment as pointing out the wrong "your" and reflexively downvoted it. I'm prompting people to see that's not the case.
Yes, consistency is good. Making things easier is good. Are you actually arguing for using "this" when it's syntactically unnecessary, or against? It isn't clear from what you wrote whether you're even taking a position either way (although, I do agree that you should pick one and be consistent about it -- I favor using "this", even when it's not necessary).
I'm arguing that C# should have made it mandatory or never required, so "sometimes unnecessary" is no longer a thing, so having to learn when it is and isn't necessary isn't a thing, and "I know when it's unnecessary and that makes me better than other developers" blog posts aren't a thing, and "I use it even when it's unnecessary because {other good reasons}" retorts aren't a thing, and that if we pushed for that kind of consistency universally, instead of admiring the cleverness of shaving 5 characters off a mountain of verbose C# or admiring the people who know when you can and can't do so, many things would be simpler, and that would be better.
For example, merging "your" and "you're" one way or the other, because everyone knows what they mean from context and they sound the same anyway, or removing "you're" in favour of "you are".
I suspect that if you go back and read my comments, you'll see me saying that it would be nice if things were more consistent, simpler, and that "minor distinctions that do nothing to help are a waste".
> Well, then you should have literally written this instead
Somewhere between victim blaming and invoking the Just World hypothesis. You introduced the idea that I should care about downvotes, you blamed my downvotes on complaining about them. Then when I denied that, you blamed the downvotes on me being accusatory. Now you've settled on the downvotes being because I'm not good enough at communication, and now you have a reason why I deserve them, you can tell me how I "should literally" have behaved if I didn't want them - a suggestion only you have pushed. That's annoyingly smug with 20/20 useless hindsight. Yes maybe it would have been good from a downvote point of view if I had literally written something that took me hours to get round to framing, instead, but I didn't, and can't, and if the cost of that is downvotes then so be it.
I'm a big C# fan, I like a lot about the language, but the inheritance is especially cumbersome and it can become a minefield for the uninitiated. It's only something you avoid when you have tripped by it. The biggest minefield is when using them in initializers, but I'm pretty sure the compilar has displayed warning for sometime and I believe Resharper has had it forever (.net 3.5?).
The only legitimate uses of 'this' is when is when using indexers and when using 'this' as the first parameter in extension methods. Nothing else springs to mind, and as mentioned above, 'base' should normally be used in indexors.
However, there are times when I have used it for readability (which could be considered as an antipattern). I guess this is an example of why languages become so complex. Semantic sugar everywhere (it's addictive!)
Extention Methods https://docs.microsoft.com/en-us/dotnet/csharp/programming-g...
Also lots of people love to add _ or m_ before fields to distinguish them from local variables, so why not just use `this.`? I think once you actually use this style it starts to make a lot more sense.
Color me odd, I did do 20+ years of Smalltalk, that weird "objects all the way down" language where self was not an option.
What I have observed anecdotally over the the last 10 years as I've become much more of a polyglot is that, very generalisticaly speaking, spheres that frown on self/this produce code that is less "objecty" than otherwise. Spheres that eschew it's use often feel like just simpler C. And to be honest, that's fine. If you thing object oriented programming is just about "better organizing/structuring" your code, then do the ALGOL style thing. But if you want to have that "my God, it's full of objects" experience, try being explicit with your message sends.
Curious how you would think of this approach. Would you call that inconsistent, or a useful separation of thought patterns?
The only problem of style is when there are mixed styles within the same codebase.
But on that note, every code base should have aggressive linters and auto-formatters that force everyone to use the same style. So the question is moot.
If you're working in someone else's codebase and their style is different than your preferred style, suck it up because it just doesn't matter.
--- When moving from a company that didn't have said linters and auto-formatters to one that did, all of these questions disappeared and we could focus solely on solving problems that mattered.
Once you get to a certain level of experience though, it should be a foregone conclusion that all code looks the same automagically.
Next you’ll say “all code should not be vendored.”
Anyway it’s interesting to disseminate opinions on style because agreement on style beyond the boundaries of a corporate engineering hierarchy is really the best route to solving style issues.
>Besides Git is ill-suited for vendoring.
VCS is ill suited for vendoring, but is there a better option? Vendoring is usually a "hack" in some ways as you're keeping dependancies local when you don't "have to", but if your goal is to keep 'em local then git does a fine job.
I'm surprised to see push-back for this idea because I've never come across this stumbling block. Please feel free to disagree, as I feel like there might be a perspective I'm not considering or something I can learn.
If anything, a Nix derivation with a hash is going to be more reliable than a git commit hash even when you control the repo because it forces the build of your vendored dependency (including any transitive dependencies) to be deterministic. The downside, of course, is that it can be a lot of effort to retrofit deterministic builds on top of external codebases not built with that in mind...
One other advantage to the "this.Foo()" syntax is that this allows using the extension method "Foo", which otherwise could not be resolved by the compiler.
(I also briefly considered if extension methods invoked on "this" would bind more strongly than base types - however, that is never the case [0])
[0] https://docs.microsoft.com/en-us/dotnet/csharp/programming-g...
I don't worry too much about these things. The whole point of something high level like C# is to be able to focus more on the problem domain than the language features. In most cases, I would just let VS clean these things up for me. I very rarely disagree with what the latest version of VS2019 suggests for code cleanup tasks.
I’d argue that calling an extension method of the class from within the class is a good indicator that said extension method should be added as a member... it’s not really an “extension” if it’s required for the class to compile.
But it’s nothing I’d lose sleep over...
https://sharplab.io/#v2:C4LgTgrgdgNAJiA1AHwAICYCMBYAUHqAQwFs...
It basically guarantees that whenever I see a symbol, I know where it's coming from.
If I see: `myFoo`
Cool, it must have been declared somewhere in this scope of code.
If I see: `this.myFoo`
Cool, it's declared as a field on the class and therefore exposed at a higher level (even if it's private, changing its value can have side effects later on outside this scope since it is persisted state on my class).
Note that this only really works when your linter guarantees it. But as a developer, I will always take a guarantee over a non-guarantee :)
I can't find when it started happening, but there is at least someone wanting to turn it off Jan 2019: https://developercommunity.visualstudio.com/idea/437785/allo...
It looks like VS2019 will only let you style static members in bold - or not - and that's it. You can't use a different color or italics - just bold or non-bold. Hmpf.
> Cool, it's declared as a field on the class
IDEs can highlight local fields and class members in different colors. Furthermore, in C# convention, member variables are preceded with an underscore (e.g. `_myFoo`).
This means there's never any confusion at all as to what is a member and what is a local.
In C#, this confusion is possible, if you name your private members the same way as your locals. Microsoft recommends and uses a convention which does exactly this. [1][2]
edit The point being that this.myThing will cause a compilation error unless myThing really is a member, giving an ironclad guarantee to the reader that it is indeed a member. Unlike in Python, in C# it's not possible to give the reader an ironclad guarantee that a local really is a local, as the implicit this cannot be disabled.
[0] https://stackoverflow.com/a/1984121/
[1] https://web.archive.org/web/20100316191542/http://blogs.msdn...
[2] https://github.com/microsoftgraph/msgraph-sdk-dotnet-core/bl...
This is also an issue with own methods versus static imports.
> Microsoft recommends and uses a convention which does exactly this. [1][2]
The article mentions this:
> I know, capitalization conventions change over time, and I could have sworn there was a time when camel casing was the recommended approach for fields (specifically private fields)
It looks like "private fields" (along with private everything) have been completely dropped from the conventions: https://docs.microsoft.com/en-us/dotnet/standard/design-guid...
Only parameters remain conventionally camelCased, and it makes sense that they remain specified as C# allows for named parameters (so parameter names are part of the public interface).
Pascal case, same as Properties so they can be seamlessly refactored.
Your link explicitly states that it does not cover private fields:
> Internal and private fields are not covered by guidelines
Which is specifically what I noted was dropped from the document linked by TFA.
> Pascal case, same as Properties so they can be seamlessly refactored.
Your link also explicitly states that it only applies to static public or protected fields, and that public instance fields are forbidden by the member design guidelines*:
> The field-naming guidelines apply to static public and protected fields […] public or protected instance fields are not allowed by the member design guidelines.
So the only "seamless refactoring" which is allowed is essentially irrelevant (it's switching between static fields and static properties).
I consider it a mistake on the part of Python to go with self rather than this, but it's a really bad idea to write code that surprises other programmers in that way.
self makes more sense though. And smalltalk used self. And so did (obviously) Self.
The most important part is to not use both (which of course is what PHP does).
> Use $this to refer to the current object. Use self to refer to the current class. In other words, use $this->member for non-static members, use self::$member for static members.
This is a different way of doing things, yes, but it makes sense.
There doesn't seem to be an easy way to detect it using external tooling, either: https://stackoverflow.com/q/957766/
I've posted a new answer there on how to configure the check in VS2019.
>But even in this case, you should really be using Pascal casing as per Microsoft's Capitalization Conventions.
Like other sibling comments stated, I also disagree. The C# compiler knows about "this" but not about arbitrary Pascal naming conventions. Therefore, it's more reliable to choose the syntax where keyboard typos can be detected by the compiler.
The blog author's advice would make more sense if C# compiler assigned semantic meaning to alpha case like Golang. In Golang, the first letter of identifiers being upper vs lowercase changes the scope of visibility: https://golang.org/doc/effective_go.html#names
Also an excerpt from Microsoft guidelines cited in sibling comment that recommends using "this." :
>Do not use a prefix for member variables (_, m_, s_, etc.). If you want to distinguish between local and member variables you should use “this.” in C# and “Me.” in VB.NET.
struct Location {
public readonly float x;
public readonly float y;
public Location(float ax, float ay) {
x = ax;
y = ay;
}
public float X {
get { return x; }
set { this = new Location(value, y); }
}
}I had to try it because I at least thought I might get a compiler/IntelliSense warning, but nope, it works perfectly and silently.
It's too hard to rely on convention to communicate details across an org, especially when using 'this' is what your devs default to (perhaps a relic from their Java days in school).
Edit: @masklinn pointed out the static imports feature, which allows you to import static methods in a such a way that they don't have to be prefixed with the class name when calling them, so I take back what I said about function calls
Not true. Both have static imports. Which means in e.g.
PI * Pow(Radius, 2);
Also
> it's either a member of the current object, or it's a local variable in the current function.
So they do have non-member variables, in local variables. Making it clear that something is not a local variable is valuable as it means the variable has a much larger scope than you might expect.
> So they do have non-member variables, in local variables. Making it clear that something is not a local variable is valuable as it means the variable has a much larger scope than you might expect.
Yes, local variables are non-member variables. I'm just pointing out that people's habits may have been built in JavaScript or the like, where a non-member variable could be:
- a global
- from a closure, anywhere up the tree
- an import from another file
Whether or not the explicitness is still important in C# code, it's at least much more important in JavaScript and Python code (in fact I'm pretty sure both languages require it)
> - a global
> - from a closure, anywhere up the tree
> - an import from another file
C# has "from a closure, anywhere up the tree" and "an import from an other file" though, the only thing it arguably doesn't have is "a global".
> Whether or not the explicitness is still important in C# code, it's at least much more important in JavaScript and Python code (in fact I'm pretty sure both languages require it)
Technically you can use the `with` statement to make it implicit in javascript.
You should not, but you can.
It could also be a field in the parent class. Protected fields are a bad idea but I’m sure some people use them.
Having said that, C# also permits you to have fields with the same name in both the parent class and the current object.
This will give you a compiler warning suggesting you use the new keyword, though.
Field access and modification are the most important part of a member to consider. Mutation is a well-recognized pattern for bugs. I want the points in the method at which mutation occurs to be highlighted. The use of "this" to distinguish fields from locals makes that possible.
If you've been programming for a few years, frequently just the field access pattern gives you an intuition of what sort of member you're looking at in just a glance.
The programmer's brain holds limited context at any time. The larger the context required, the less detailed the understanding. Being able to predict what a method does in a glance means the developer better preserves their current context.
I strongly avoid inheritance, so I rarely have use of "base". Again, when I'm explicitly invoking "base", I really want that to stand out, because it's unusual.
(For this same reason, I also discourage explicit use of the "private" access modifier. Everything should be private, except for the few things that must be public. Extra access modifiers add noise, which distracts from the important information.)
Finally, I strive to make my code read as fluently as possible. In my experience, fluent code makes it harder for bugs to hide. The ability to clearly express a concept in a spoken language correlates very highly with the ability to implement that concept in a program without errors.
Compare:
if (AllChildOperationsAreComplete) { StartNextOperation(); }
if (this.AllChildOperationsAreComplete) { this.StartNextOperation(); }
The extra hiccoughs in the expression of the second form are each interruptions, each interruption increasing the chance that you drop some context while reading the code.
I do this all the time, and I didn’t know there were rouges out there who I angered by doing it. It may not have been the intent of this.x, but for totally superficial reasons, has become the de facto way of knowing you’re calling a member property/field/method, and not having to bother with memorizing some style guide’s idea of how you should represent these things.
Also, just for speed of development, say I want to call a method but I don’t know the exact name, typing “this.” gives me an intellisense list of everything available in a compact, scrollable list. Makes my life a lot easier.
public class MyClass : YourClass {
public void MyMethod() {
if(!base.YourMethod()) return;
}
}
My rule is exactly the opposite. Only allow `base` in:
- Constructors
- Overrides
And I agree it is dangerous - going against conventions on giant projects with dozens of developers, is dangerous. Should have left off the base. and the this. everywhere, like Visual Studio recommends.
What does being a manager have to do with code style? This is textbook micromanagement.
> But even in this case, you should really be using Pascal casing as per Microsoft's Capitalization Conventions
Not sure what this is refering to. In assignmenmt this.foo = foo, both the field and the parameter should have the same casing, that's why "this.foo" is needed to distinguish the field from the parameter.
In the end, these are mostly cosmetic/style choices. Consistency matters more than whatever style is chosen, but I of course find it very important that we agree on my preference.
I understand his suffering :)
Amongst other gems was a nice reminder about Self's innovation in eliminating the 'self.' prefix (or equiv). It's in the name, right? Can't believe I forgot that.
I'm surprised no successor (to Self, Java, or both) has eliminated (disallowed) variable aliases. So that local and instance variables cannot have the same name. Voila. One category of errors completely eliminated.
--
Any and all use of 'this.' prefixes is a full stop hard omega fail. For both the language and any user of that language.
Whatever else I think of JavaScript, the lack of intrinsic identitiers (name?) should have immediately disqualified the language and prevented any further consideration.
Huge disappointment that TypeScript did not address this original sin. (Haha, nerd joke, I slay me.)
This OP gives example of this.method(...). What? Is that a C# thing? Why would anyone do that?
Oh well. Let's go back to arguing about dynamic vs static typing.
My personal preference for C#: private fields are camelCased and are always prefixed with this on access (disambiguating field from parameter access); methods and properties are not prefixed. I find this gives a nice balance between code noise and explicitness.
Same principle could be applied to method access (since C# has local methods and static imports) and I would happily concede that it is a sane approach - but I don't find the code noise it produces is worth it.
If I join an existing project that consistently uses some other approach (even if it's an old-school underscore-prefixed fields approach) that's OK too.
Then set it as a global rule in .editorconfig (or whatever equivalent tool you’re using) and never think about it again
I also skip brackets with single line if statements and many other optional explicit things you could do but add unneeded text to the code, seems strange to not use language features to make the code more terse.
If its not good practice I wish they would just take the feature out of the language and make everyone run a linter before upgrading to the new more explicit language version for conversion.
Developers would be wise not to mistake reified opinions with correctness.
There's nothing sacred about Visual Studio defaults; give me an IDE and a language I use regularly, and there's probably at least one default style preference I disagree with (and typically more, especially for C-like languages where the currently fashionable defaults place a newline before every keyword and brace).
As for this, I'd actually prefer a Python-like convention making the instance explicit in instance method parameter lists and bodies, but, given that implicit this exists in the first place, I don't really have a preference, and therefore stick with the conventional default — omit unnecessary this — unless working on an existing project whose established style includes it.
But that is disingenuous. We aren't talking about [insert opensource language] + [insert opensource IDE], where your the IDE's inbuilt preferences match closer to the IDE developer's preference than the random language you are developin in it.
This is Microsoft's Visual Studio and Microsoft's C#. Their linter is recommending the best practice conventions.
You can definitely disagree with them, but I don't, that is why I wrote this piece. I don't like "this." scattered throughout the code when it isn't proper according to the language designers.
Personally I think having a consistent style is more important than the details of that style, on the balance of things (use your judgment, this ain't a strict rule) and I love that we have tools like StyleCop.
Is it just a matter of taste?
Don't get me wrong, it often annoys me to see it over used, but it isn't incorrect, and the assertion that visual studio recommends removing it isn't really strong supporting evidence, as you know it recommends changing plenty of things, that are perfectly acceptable.
Similar to people who use a prefix like "m_" or an extra underscore or capitalization scheme for member names.
These are all matters of opinion and style and no one thing is correct.
My point was that the same people who insist that 'this' is superfluous because "IDE can handle it" are also advocating use of "m_" because of "visual signal". I simply think 'this' is superior to "m_" in every way.
Because then I can tell it's a method by only looking at the call site.
Now, if you want an example of incorrect C#, I'm sure I can find something I've got running in production much worse than explicit this...
Posting from mobile, so formatting might have issues, but below I'm monkey patching a bug out of the immutable AST Entity Framework generates when using SQL Server's DateDiff function. On the one hand it's truly monstrous. On the other hand, it's saved hundreds of lines of code elsewhere in the application.
public static IQueryable<IGrouping<TKey, TSource>> GroupByDateDiff<TSource, TKey>(this IQueryable<TSource> source, Expression<Func<TSource, TKey>> keySelector) {
var body = (NewExpression)keySelector.Body;
foreach (var arg in body.Arguments) {
if (arg.NodeType == ExpressionType.Call) {
var callNode = (MethodCallExpression)arg;
if (callNode.Method.Name == "DateDiff") {
var dateDiffFirstArg = callNode.Arguments[0];
if (dateDiffFirstArg.NodeType == ExpressionType.Constant) {
//It was already a constant, so we're good.
}
else {
//HACK: This will break if the internal implementation of ReadOnlyCollection changes.
var listInfo = typeof(ReadOnlyCollection<Expression>).GetField("list", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance);
var list = (IList)listInfo.GetValue(callNode.Arguments);
if (dateDiffFirstArg.NodeType == ExpressionType.MemberAccess) {
list[0] = Expression.Constant((string)GetMemberValue((MemberExpression)dateDiffFirstArg));
}
else {
throw new ArgumentException($"{nameof(GroupByDateDiff)} was unable to parse the datePartArg argument to the DateDiff function.");
}
}
}
}
}
return source.GroupBy(keySelector);
}- In current class
- In a base class
- A static method in the current class
- A "static import"ed helper method from outside the class
In the past I'd say you could use it to be self-documenting, but these days you have semantic colorizing, tooltips, etc., for this sort of discovery if you want it.
If your call is implemented in the current class, the 'this' is implicit.
If the call is implemented in a base class, and not overriden, the base class method is still available through the 'this' keyword. If you do override the base method, you must explicitly do so. Why you would want to call the base in a class which overrides it, I don't know.
Static methods in the same class are impossible. You can't have two methods with the same signature, static or otherwise.
Extension methods are different, but there is clear precedence. The instance method is used before the extension. 'this.ExtensionMethod()' is still valid, so you aren't getting any of this referential transparency.
In any case, the only use case for 'this' is to call extension methods from within a class the extension is defined for. It certainly does not give any referential transparency.
> Static methods in the same class are impossible. You can't have two methods with the same signature, static or otherwise.
I never said anything about having both at once -- I was talking about refactoring - changing the location of where the fn was defined - from a method to a static. (As one does if it turns out you're not using any object state.) Of course, some languages let you call static's on the same class with 'this' and some can only reference instance methods.
> I've had managers want developers to use this to signify calling a base class's method, which is incorrect. You should be using base.
public class MyClass : YourClass {
public void MyMethod() {
if(!base.YourMethod()) return;
}
}I advocate using base for base.
I advocate against using this, as does Visual Studio 2017 and above [1].
Which isn't correct. Lets say someone were to come in and override the base class's method, now calling this.SomeMethod() would not be the intended behavior of the original code that was improperly using "this." as "base."
I don't want "base." or "this." in my code at all unless there is some incredibly rare case that I MUST use it.