
Using 'this' incorrectly in C# - jermaustin1
https://jeremyaboyd.com/post/you-are-using-this-incorrectly
======
nend
>I've had coworkers who would put this before every method call, because...?
When calling your own method, in your own class, that inherits/implements no
other classes, why on earth would you decorate your own method call with this?

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.

~~~
throw1234651234
It's superfluous.

~~~
masklinn
That it's _technically_ superfluous doesn't mean it's _practically_ useless.

~~~
velox_io
How did the article not mention extension methods, when that's arguably the
only true use case (many are against such methods).

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...](https://docs.microsoft.com/en-
us/dotnet/csharp/programming-guide/classes-and-structs/extension-methods)

------
tantaman
I hate all of these rants on questions of style.

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.

~~~
cjfd
I think there actually are bad styles. The google style guide is such a bad
style guide. The combination of 2 spaces indent and not putting { and } on top
of each other makes it difficult to see at a glance where a block begin and
ends.

~~~
neurotrace
I wholeheartedly disagree. Two spaces and the One True Brace style is my
preferred way of working. Blocks are easy to identify based on indentation.
This is exactly why auto-formatters are the best solution. Then there's no
need to have this conversation. Some of the style choices we have at work are
not ones I would have chosen but I'm quite comfortable with them now thanks to
auto-formatting

------
ThePadawan
Whenever I read articles like this, I happily reflect that C# is such an
excellent language to work with that these tiny complaints (I would hesitate
to call them "issues") are the only ones popping up.

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...](https://docs.microsoft.com/en-
us/dotnet/csharp/programming-guide/classes-and-structs/extension-
methods#binding-extension-methods-at-compile-time)

~~~
cgrealy
> 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’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...

~~~
wvenable
The extension method might be on an interface of the class and not on the
class type itself. That's really the only time I've ever had to use this.

~~~
cgrealy
Fair point.

~~~
ThePadawan
Note that this is also how can "cheat" to get default but overrideable
interface methods (before they were introduced in C# 8):

[https://sharplab.io/#v2:C4LgTgrgdgNAJiA1AHwAICYCMBYAUHqAQwFs...](https://sharplab.io/#v2:C4LgTgrgdgNAJiA1AHwAICYCMBYAUHqAQwFsBTAZwAdCBjUgAgGUB7MgYQnOFYEsAvQgCMANqQAyPQWEJgAnngDeeeivqoAzPR5RgpMADNaDFmQCSOvYbrLVS3Kodad9AEIz6C+gHNSwANz0AL42jk7A9ABizMwAFACUfiFBSUkaapgAbGro9AAipIYQwsAAogAeulDkPMxVSXahKmmomWH0ALKEXjw0McAAFjzkTKyk5roGRvRlcUkODY0OqADs0wB0bmCJ9o3BOyp7ewQkFNR09BJSMrIA8lCkiqkADCPsnNzE/EKil9Jy2w4kgB6IH0ACqUBorDIzgGQ3o3HozAAbnowDw4AwBgxMYVimFLFMyANmHB6AB3HgDeiyZgQMD0GjvXgCYA1KprYEAKlSmhaWQwHVu5Kg5Uq1Vq5Hqc1UzVa2nCnW6vThwxMYwsk3OMxlKgWiyaq3VHC4LO+4kkf1ka3yeNKFVIVXZ5DWSp6MRm9C59HQAN2ST2Di5QJS+zUfJy6tMxEownoIFeGomVgeYf1jjSCtc7gAvAA+egAVj9Kl14baUViszD8zLS1WqtdXXdCTLgdUhzwxzIVCm7Q8qXQmnTS3QORHjRaAE4YvdyRdLdc7qQ1lGY8J4mtK/FWzWOwG8IEgA===)

------
2bitencryption
I like the explicitness of 'this'.

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 :)

~~~
vsareto
I've never liked added explicitness for readability in tool heavy environments
(which C#/VS is) since it tends to not add much value because those tools can
impart the same information. If we were doing C# in a really basic text
editor, I could understand the need for qualifying 'this' though.

~~~
DaiPlusPlus
Visual Studio (without third-party extensions) still doesn't offer a way to
color/highlight/style static member call-sites differently to instance member
call-sites, that's why I always prefix instance member access with `this.` and
why I always give static fields a name with a leading underscore.

~~~
accoil
Doesn't Visual Studio make static/const members bold these days?

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...](https://developercommunity.visualstudio.com/idea/437785/allow-
to-turn-off-bold-keywords-with-enhanced-colo.html)

~~~
DaiPlusPlus
Ah, yes - that's a new setting in VS2019 (I use VS2017 as my daily driver, but
sometimes use VS2019).

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.

------
MaxBarraclough
In Python, the only way to access members of the current object is by
explicitly using _self_ (which should really have been called _this_ in line
with other languages, but that's another matter). [0]

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/](https://stackoverflow.com/a/1984121/)

[1]
[https://web.archive.org/web/20100316191542/http://blogs.msdn...](https://web.archive.org/web/20100316191542/http://blogs.msdn.com/brada/articles/361363.aspx)

[2] [https://github.com/microsoftgraph/msgraph-sdk-dotnet-
core/bl...](https://github.com/microsoftgraph/msgraph-sdk-dotnet-
core/blob/bc4c8874637/src/Microsoft.Graph.Core/Requests/Upload/UploadSliceRequest.cs#L20)

~~~
c22
You don't have to call it _self_. You can name the reference whatever you
want, _this_ works just as well. Though perhaps it's not as pythonic?

~~~
MaxBarraclough
I hadn't wanted to get into the weeds of Python, but yes. You _could_ use any
identifier you want, but breaking with convention in this way is not a wise
move.

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.

~~~
masklinn
> I consider it a mistake on the part of Python to go with self rather than
> this

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).

~~~
m_t
Yeah but to quote stackoverflow:

> 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.

------
jasode
_> , but the closest I will get to accepting it is in a constructor where you
are passing values that will be stored as private fields. int maxAmount;
SomeClass(int maxAmount) => this.maxAmount = maxAmount;

>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](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._

------
mwkaufma
The author fails to mention the only truly-abusive us of this: working around
readonly variables:

    
    
      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); }
        }
      }

~~~
Someone1234
That's evil. I'm both impressed and horrified by that trick.

I had to try it because I at least thought I might get a compiler/IntelliSense
warning, but nope, it works perfectly and silently.

------
dhruvmittal
I'd argue that this.myValue = myValue is a lot clearer than the 'conventional'
MyValue = myValue, unless you live in a world where your linter can mandate
pascalcase for private fields always (and never otherwise).

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).

------
brundolf
Everyone's chiming in with "you should always use 'this' because it's more
explicit", which I personally tend to agree with. But there is a dimension to
this that may not be obvious to people who don't come from Java/C#: these
languages don't have non-member functions or variables. So if you're accessing
a variable without [obj]. it's either a member of the current object, or it's
a local variable in the current function. If you call a function without
[obj]. it's unlikely (maybe impossible? I'm not certain) that it _isn 't_ a
method of the current object. On top of that, by idiom Java and C# code tends
to be dramatically more verbose than many other languages. So I could
understand the desire to cut that down some when there would also be less
ambiguity than in, say, JavaScript.

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

~~~
masklinn
> these languages don't have non-member functions or variables

Not true. Both have static imports. Which means in e.g.

    
    
        PI * Pow(Radius, 2);
    

all three symbols may or may not be part of the current class. In this case,
only Radius is, the other two come from `using static System.Math` (it's
straight from an official example for the `using static` directive).

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.

~~~
brundolf
I didn't know about static imports. Technically those are still member
functions, but you are able to call them without a prefix, so I retract that
point.

> 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)

~~~
masklinn
> 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

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.

------
drfrank
I discourage the use of "this" except for field references, in C#.

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.

------
kolbe
> I've had coworkers who would put this before every method call, because...?
> When calling your own method, in your own class, that inherits/implements no
> other classes, why on earth would you decorate your own method call with
> this?

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.

------
sebazzz
> 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;
            }
        }
    

That is some dangerous advice! If you later decide to override the base class
method, you're in for a treat! The non-overridden behavior is then used.

My rule is exactly the opposite. Only allow `base` in:

\- Constructors

\- Overrides

~~~
jermaustin1
That is perfectly fine, but in this case, the manager wanted this to
explicitly signify base, If you didn't provide this, it meant it was a MyClass
method.

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.

------
tantalor
> I've had managers want developers...

What does being a manager have to do with code style? This is textbook
micromanagement.

------
alkonaut
Prefixing "this.field" is nicer looking than "_field" for signifying a field
rather than a local or parameter (These days most syntax highlightrs should be
clever enough to distinguish this by color, but in the past this was useful.
As an added benefit autocomplete will be triggered after "this.".

> 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.

------
jermaustin1
Wow. I didn't think this post would cause so much controversy, since these
"opinions" are shared by Visual Studio 2017 and above [1]

1: [https://i.imgur.com/9F6Lyx8.png](https://i.imgur.com/9F6Lyx8.png)

------
oaiey
While I have seen this behaviors, they are not the norm at all (>100 devs). C#
style is avoiding this keyword and tools (resharper etc) are even highlight
unnecessary use.

I understand his suffering :)

~~~
crc32
StyleCop on the other hand - a freely available linter as opposed to the
commercial resharper - mandates the use of `this` by default.

------
nurettin
It isn't about consistency or style. People use "this." to start their
intellisense autocomplete which brings up the most used methods depending on
the context.

------
specialist
I recently listened to a 2015 interview with David Ungar, of Self, Smalltalk,
and misc fame. [https://www.youtube.com/watch?v=8nfrC-
YLYqc](https://www.youtube.com/watch?v=8nfrC-YLYqc)

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.

------
balfirevic
Lot of personal preferences masquerading as objective truths in this article.

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.

------
cgrealy
Honestly, I really don’t care. It’s (at most) a 5 minute conversation between
leads on a new repo. Either use this. or an _ prefix or whatever.

Then set it as a global rule in .editorconfig (or whatever equivalent tool
you’re using) and never think about it again

------
SigmundA
Yeah I don't use Fully qualified class names nor this unless I have to. If I
have any question about the scope a quick hover tells me everything I need to
know.

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.

------
dleslie
In C#, 'this' is how you gain access to extension methods for the current
class. At least, that's how you get most tooling to expose them as viable
completion candidates.

~~~
asdfman123
I sometimes type "this." to get some help from Intellisense but I then delete
the prefix after I find what I need.

------
programmarchy
If it was incorrect, the compiler, or at least the linter, would throw an
error or at least a warning, no?

Developers would be wise not to mistake reified opinions with correctness.

~~~
jermaustin1
Visual Studio 2017 and above actually does complain about it. It will
recommend removing `this` from your code.

~~~
jasomill
That, or change the IDE style preference that triggers this suggestion to suit
your preferred style, preferably using a mechanism like _.editorconfig_ that
embeds these preferences within the project itself.

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.

~~~
jermaustin1
> give me an IDE and a language I use regularly, and there's probably at least
> one default style preference I disagree with

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.

------
klodolph
Last time I worked in a C# code base I made the entire code base StyleCop-
clean. StyleCop has a rule that you are supposed to use an explict "this",
although it may only be for field access (I can't remember).

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.

------
elboru
I can think of some readability benefits of using ‘this’. But the author
didn’t give further details on why he didn’t like it.

Is it just a matter of taste?

~~~
jermaustin1
It is about tastes I've had most of my C# developing life. Taste's that
coincidentally align with the conventions of Microsoft Visual Studio's built
in linter.

------
Meph504
You mistate "incorrectly" for "in a way I don't like. "

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.

------
asveikau
I had a colleague who would like to put "this" on member accesses so that it
was absolutely clear from a glance that it was a member access and not
something local.

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.

------
jagged-chisel
I feel like this is an excellent area for a post-checkout / pre-checkin script
to process code to / from the style guide's recommendations and your own
personal preference. After all, the machines are here to serve us, not us to
serve them.

------
self_awareness
I know lots of programmers who discourage using 'this' when accessing class
fields, because it's "not needed", but at the same time use "m_" prefix for
fields.

~~~
idemprotent
The convention for using m_ prefix is used to signal visually what is a
private member and also p_ prefix is for params. I don't like it that much and
I've seen it both in Java and .Net but, if I'm working on a code base which
uses this convention I;ll simply stick to it. Consistency is nice,
inconsistency not so.

~~~
self_awareness
Well, this.field also can be used to signal visually what is a private member
or what is a private method. And it's supported by the language, and there's
no need to use such magic prefix strings.

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.

------
icholy
> I've had coworkers who would put this before every method call, because...?

Because then I can tell it's a method by only looking at the call site.

------
caseymarquis
The author is complaining about explicit use of 'this' where it's not
required. It's hardly incorrect, it's a preference. I'd personally pick
explicit use of this if I cared enough be consistent about it. I've
specifically been bitten by refactoring a local variable to the same name as a
private variable on accident, and shadowing the private variable within the
local scope. Using explicit this prevents that from happening.

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);
            }

------
thelazydogsback
"this" is not just superfluous -- if you use it, you lose the referential
transparency of the call. If you don't use "this" you can change where your
call is implemented to any of these places without refactoring all the calls:

\- 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.

~~~
ddek
No?

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.

~~~
thelazydogsback
I'm not sure I'm following your objections at all - if this is implicit, then
you don't need it, correct?

> 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.

------
theandrewbailey
Does using 'this' everywhere break subclass overrides?

~~~
sebazzz
No, but using `base` does, and he advocates for using `base`:

> 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;
            }
        }

~~~
jermaustin1
I don't advocate for using base instead of this.

I advocate using base for base.

I advocate against using this, as does Visual Studio 2017 and above [1].

1: [https://i.imgur.com/9F6Lyx8.png](https://i.imgur.com/9F6Lyx8.png)

~~~
bradjohnson
You're advocating using base.foo() rather than this.foo() which is not
equivalent behaviour and is bad advice.

~~~
jermaustin1
Incorrect. I am advocating that if you want to signify you are using the base
classes method, use base. Otherwise don’t use base or this.

------
ashleyn
_laughs in JavaScript_

