Skip to content

Add "this." prefix to all field accesses - #309

Merged
Sergio0694 merged 4 commits into
mainfrom
dev/fix-fields-this-qualifier
Jun 13, 2022
Merged

Sergio0694 merged 4 commits into
mainfrom
dev/fix-fields-this-qualifier

Conversation

@Sergio0694

Copy link
Copy Markdown
Member

This PR adds the missing this. prefix to all leftover field accesses.

@Sergio0694 Sergio0694 added the maintenance ⚙️ Some regular maintenance updates label Jun 11, 2022
@Nirmal4G

Copy link
Copy Markdown
Contributor

Seeing the code, we'll know whether it's a parameter from the method or a field and a property within the class/struct. Do we have to add these? It is visually distracting. IMO, it's best to remove all this qualifiers unless the names of the method and parameter are the same which is rare and compiler would certainly complain about it.

So, I think it is safe to remove all this! On that note, we should also come up with a definite way to name fields. Whether prefixing _ underscore is required or not. There are a lot of inconsistencies here. I vote NO but some .NET projects use them!

@Sergio0694

Copy link
Copy Markdown
Member Author

Fields should just be named lowerCamelCase. The only reason why some use _lowerCamelCase is that they're from old classes we haven't refactored yet. I guess I might as well update those too while at it, since it's not a breaking change anyway.

"Seeing the code, we'll know whether it's a parameter from the method or a field and a property within the class/struct. Do we have to add these? It is visually distracting."

Adding this. makes the code less error prone, as it avoids the risk of accidentally referencing a local or a parameter with the same name. Yes, you can manually add this. when there's a conflict, but that is precisely the point - making this mandatory means you don't have to rememeber, and you can't make mistakes. If there's this, it's 100% a field.

@Nirmal4G

Copy link
Copy Markdown
Contributor

Yes, you can manually add this. when there's a conflict, but that is precisely the point - making this mandatory means you don't have to remember, and you can't make mistakes. If there's this, it's 100% a field.

Guess it makes sense! Still, I personally hate it. I just use different names for parameters, locals and fields.

@Sergio0694
Sergio0694 merged commit 7fdc18e into main Jun 13, 2022
@delete-merged-branch
delete-merged-branch Bot deleted the dev/fix-fields-this-qualifier branch June 13, 2022 15:51
@Balkoth

Balkoth commented Aug 5, 2022

Copy link
Copy Markdown

Theres this directly from the runtime team.

@Sergio0694

Copy link
Copy Markdown
Member Author

That is not the official guidance for all C# developers, just the style convention the runtime uses 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance ⚙️ Some regular maintenance updates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants