stride icon indicating copy to clipboard operation
stride copied to clipboard

_underScore for private fields, anyone? Team assessment.

Open najak3d opened this issue 2 years ago • 7 comments

Most projects I encounter have adopted the "" prefix for private fields, to differentiate from local variables and method parameters. Not only does this provide a more clear differentiation when you look at it, but also separates the Intellisense very nicely... if you want a field, type "" and it shows you your options, while using just pure camelCase for fields, interleafs the Intellisense auto-complete with random other options.

I often define a local method parameter to "snapshot the field value" (in case of multi-threading, you don't want it to change on you halfway through the method)... and this convention also butts it's head against camelCased fields (e.g. "var mapMode = this.mapMode" is worse than just "var mapMode = _mapMode")

If you have BP's set or debugger stops on a line, it worsens the color differentiation as shown here:

image

=== So I was just wondering what the current thinking is of the Stride Team on this. Are we split 50/50, or is there a growing sentiment that we'd like to move towards "_underscore" field naming? If the team is split, then there's no point on my continuing any form of rant or argument here.

I may be planning on working intimately with Stride3D, possibly until I die (I'm 51 yrs old now, and planning to retire in the near future to dedicate time to personal game/app development, or tooling for gaming) -- and so just want to get a quick assessment of the thinking from the current Stride Team on this topic, see what type of hope there might be for a future refactor.

najak3d avatar Mar 20 '22 21:03 najak3d

I prefer underscore but this is kind of nearing bikeshedding territory, we can't change it at this point as it'll just pollute the git history and having a mix of both is worse than sticking with what we have.

Eideren avatar Mar 20 '22 21:03 Eideren

I'd argue that having it mixed is not worse - in that "_" prefix is never unclear, so when you see it, it's not confusing.

The git history thing is the biggest concern. But when I do refactorings of this nature, if the ONLY THING changed in check-ins are harmless field-name-refactors, then it amounts to no-real-change for each such check-in. Then those check-ins won't be suspected for introducing defects, other than if serialization/reflection somehow was referencing the old field names (and then that issue will also be clear).

If this were going to be done, there is no better time than the present, when usage of Stride is still light. I think a year from now, we're going to see usage of Stride3D in apps rise significantly - esp. if it's integrated into MAUI, and also works with Avalonia.

I am an odd autistic/obsessive type person for many things -- and I would find nothing more thrilling that going through 100% of Stride code and refactoring to "_" prefixed field-names. I'm guessing someone has already scripted such refactors that can be applied solution-wide if desired. Just saying -- if the team is open to it, I'd be (sickly) thrilled to be assigned this task to be applied project-wide.

And so it also comes down to the general belief of the current Team. If there is much opposition to it on a conceptual basis (some think that camelCase field names are better than '_' prefix), then that also makes this a no-go.

I'd rather not be looking at camelCase fields for the rest of my life (that's my confidence-level in Stride)... and so would consider it a worthy task to get it out of the way now, before Stride usage goes way up.

najak3d avatar Mar 20 '22 23:03 najak3d

Also keep in mind that I think it's our goal to attract more talent towards Stride3D upkeep/enhancement... so that may also factor into the importance of having a more widely accepted field naming convention, as new comers are more likely to find it appealing.

najak3d avatar Mar 20 '22 23:03 najak3d

Personally I'm not a fan of underscores and especially dislike m_ prefix I saw in some codebases. I'm sure every contributor has a slightly different stance on that - as we're quite varied in terms of background and experience. I for one use trailing underscore on local repeat names (where you've put 2 in the screenshot) as it's a rather rare scenario for me. To Eideren's point on git - the main issue for refactoring like this is killing blame - which can quickly show you the commit which last touched a given line of code.

manio143 avatar Mar 21 '22 14:03 manio143

@manio143 - Does the slight color code difference (light cyan vs. white) help you know the difference?

I vehemently agree that "m_" is a bad idea, and inferior to "" for two big reasons: it's 2-letters instead of one, and also because it alphabetizes them smack dab in the middle of intellisense, where the "" alone either puts them at the start or end of intellisense.

If I started working inside Stride3D source code, to make this less of an issue, I would need to be changing the IDE's color codes for local variables/parameters to be something with more contrast to the white, to overcome this handicap created by the overlap in naming conventions (for two very different things!).

To continue my argument, I'm gonna post a few other instances of where this overlap in naming convention can be a problem:

Imagine a longer method with parameters/variables that match a field name (as they often will) -- but then you either change that variable name or remove it -- the code STILL COMPILES just fine -- but you're method is now modifying the fields, instead of a local variable (that no longer exists).

In the case of Compiler directives, there are times when the "name refactor" misses variable instances that are stubbed out by compiler directives. So later, when you alter the DEFINES, and it activates previously stubbed out code -- instead of getting a compiler error, it compiles defectively, not warning of any issues.

ALSO when pasting code into contexts where there is no IDE color-coding. Here you lose the clarity, without color-coding to help you out.

One of the biggest reasons I enjoy "_" prefix is intellisense... typing "_" gives me a succinct list of viable options, and filters out all of the similar-named local variables/parameters. This prevents insertion of a defect from the onset, as well as makes it easier to find what you are looking for.

I think one of the biggest reasons to changeover is for sake of Stride3D long-term acceptance and uptake. I believe we're near the beginning of seeing a large uptake in Stride3D usage in real apps and games.

I believe these are the reasons that "_" has become the most common standard. I still hope that such a refactor gets approved, and that those who prefer no-underscore would be OK with this change.

najak3d avatar Mar 21 '22 17:03 najak3d

Since modem syntax highlighters can tell you what kind of identfier you are looking at by color coding it, I believe this discussion is a waste of energy. If you contribute to a big repository, you follow their style. And stride has no underscore. If @xen2 doesn't oppose, I'd like to close the issue as it's just stealing precious time and this discussion is nowhere near the importance of other areas in the source code...

However, it's not a huge problem to have the underscore convention for new code, it's just a bit odd to have two styles, but I think people will manage to understand that.

tebjan avatar Mar 21 '22 18:03 tebjan

@tebjan - you are probably right, and if @xen2 approves of closing - I will be done with this proposal entirely. I thought it was worth one big push upfront, before conceding to retire the idea.

I do agree that introducing "_" for new code will never be confusing, and I believe the majority of people, when they see it will be "pleased" rather than annoyed/confused, since it seems to me to be the most popular convention by far. I'd like to vote for the idea of allowing it for new code/features/modules, etc; even if we don't approve refactoring the whole Stride engine naming.

najak3d avatar Mar 21 '22 19:03 najak3d