RFC: To use static classes instead of structs for storing template GUIDs

[sergey-krivchenko]

Hi. Is there a certain reason why structs were chosen to hold GUIDs of Sitecore templates and fields instead of static classes?
There are a few considerations I thought of:

1. It is possible to instantiate a struct, which can be confusing to allow. With static classes, the compiler will not allow to do that.

2. Microsoft recommends using static classes to define constants. Currently, the Helix documentation shows that the conventions define to choose only structs to clearly signal the Templates type’s unique function as a constants holder only. I believe it is, indeed, very important to indicate that, though a static class can fit here even better. In Microsoft's programming guide on how to define constants they actually suggest to use static classes. For even further description of it's purpose, class name qualifier can be made very self-descriptive, for example "TemplateConstants".

Furthermore, it can also be controversial that structures are used to hold constants only, according to this discussion on SO. In some other programming languages it could be true (like C/C++), but in C#, structures can have properties, methods and even implement interfaces.

3. memory usage, size and copying aspects - a general recommendation for a structure instance size is to be less than 16 bytes, mostly because of copying aspects that can negatively impact the performance. Even though it is very unlikely that someone would decide to assign  the Templates structure to a variable (or to pass it to a method), still it does not seem to be prudent not to adhere to Microsoft's recommendation  to avoid defining a structure when it has an instance size more than 16 bytes. In our case, the structure contains only reference types and field-by-field copy is made (only references, not the objects itself). Considering that one object reference is 8 bytes in x64, usually, this recommendation is not met.

4. I've asked about this on #helix-habitat on Sitecore Community Slack and there was common agreement among the community members that static classes are more suitable for such purpose . And I received recommendation to open an issue for that.

Do you think that indeed it will be better to use static classes to hold template GUIDs, instead of structs? Should I create a pull request for this?

Thank you.

[nickwesselman]

Hi @sergey-krivchenko ... I think we absolutely should make this change. I see your Sitecore/Habitat#471 pull request, which likely won't get merged as we are trying to EOL Habitat itself. But if you wanted to do a PR for this repo for a language change, I'll be updating to reference the forthcoming Helix Examples sites.

[sergey-krivchenko]

Thanks @nickwesselman for accepting the RFC and creating a pull request for it. I believe the next sentence should also be changed:

The conventions define this as a static class to clearly signal the Templates type's unique function as a constants holder only, following the conventions for Constants in the Microsoft C# Programming Guide.

Using a static class does not indicate that it is used only for holding constants. Usually a class name qualifier is used for this (e.g. static class Constants). I think the sentence should just say that it is a recommended way to define constants, following the conventions for defining constants in the Microsoft C# Programming Guide. An explanation for this can be that static classes can only hold static members, including constants, and it cannot be mistakenly instantiated or derived from.

Should I create a pull request for this small change?