Skip to content

Conversation

@forki
Copy link
Contributor

@forki forki commented Aug 24, 2016

No description provided.

arr

member this.GetArgumentAndTypesAsArrays
member __.GetArgumentAndTypesAsArrays
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@forki @dsyme
I am not a fan of this change, I know you have used it in other cleanup PRs, however, I don't recall a discussion where this was the agreed style.

If we were to agree to a common style for this my preference would be in fact be 'this', since that is what it in fact is. I assume that you want to indicate that the this pointer is unused, but that is not an entirely interesting distinction, given that we are not writing in C# and we are immutable by default. Anyway ... let's have a discussion about a coding standard for this case before we carry on with the clean up.

Kevin

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obviously I'm pro __ as this is what many projects in the ecosystem use. (also suggested by FSharpLint)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another reason people tend to replace this. with __. is when the identifier isn't used it comes up as an unused declaration

while __. is clean.

If this/self isn't being used it's just visual noise, like superfluous parentheses.

@KevinRansom
Copy link
Contributor

@cloudRoutine
The unused declarations have it. Although I wasn't aware we did assignment analysis on the 'this' id.

For my education, I just checked Visual Studio doesn't do this even at warning level 5 is this a feature of powertools or am I missing something?

@cloudRoutine
Copy link
Contributor

yea it's a VFPT feature

@KevinRansom
Copy link
Contributor

@cloudRoutine It's not a feature I would find terribly useful. Although I have to say the whole naming the 'this' seems a bit weird to me as well. I think I would prefer a feature that complained if I named the 'this' differently on any member within the class.
Since at least that way within a class I would be able to conveniently read through it and translate the this at each different member.

I will pull this then since it is a power tools feature, that I suppose many community members have enabled. Despite my personal preferences.

Thanks for the clarifications

Kevin

@KevinRansom
Copy link
Contributor

@forki Thanks for this contribution

Kevin

@KevinRansom KevinRansom merged commit ec32fa2 into dotnet:master Aug 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants