dotnet / csharplang

The official repo for the design of the C# programming language
11.62k stars 1.03k forks source link

Enum extensions #3491

Open OrbintSoft opened 4 years ago

OrbintSoft commented 4 years ago

I want something that makes simpler to extend an existing enum without replicate values. I am not proposing inheritance, but just a syntax that allows to avoid code duplication and makes casting simpler and safer.

As sample we can have a base enum:

    public enum PrimaryColor: long
    {
        RED = 0xFF0000,
        GREEN = 0x00FF00,
        BLUE = 0x0000FF,
        BLACK = 0x000000,
    }

and an enum that extend that:

    public enum Color extend PrimaryColor
    {
        WHITE = 0xFFFFFF,
        // BLACK = 0x000000, Not Allowed, This name has already been defined.
        CYAN = 0x00FFFF,
        VIOLET = 0xFF00FF,
        YELLOW = 0xFFFF00,
        LGRAY = 0xC0C0C0,
        GRAY = 0x808080,
        TEAL = 0x008080,
        PURPLE = 0x800080,
        OLIVE = 0x808000,
        MAROON = 0x800000,
        DGREEN = 0x008000,
        NAVY = 0x000080,
        AGAIN_BLUE = 0x0000FF //Not a problem, different name, same value
    }

The Color enum is converted by the compiler into a new enum that is a merge with PrimaryColor:

    public enum Color: long
    {
        RED = 0xFF0000,
        GREEN = 0x00FF00,
        BLUE = 0x0000FF,
        AGAIN_BLUE = 0x0000FF,
        BLACK = 0x000000,
        WHITE = 0xFFFFFF,        
        CYAN = 0x00FFFF,
        VIOLET = 0xFF00FF,
        YELLOW = 0xFFFF00,
        LGRAY = 0xC0C0C0,
        GRAY = 0x808080,
        TEAL = 0x008080,
        PURPLE = 0x800080,
        OLIVE = 0x808000,
        MAROON = 0x800000,
        DGREEN = 0x008000,
        NAVY = 0x000080,        
    }

This also creates an implicit casting from PrimaryColor to Color.

        public static void DoSomething(Color color)
        {
            Console.WriteLine(color);
        }
DoSomething(Color.VIOLET); //It's fine

DoSomething(PrimaryColor.RED); //This simpler syntax involves casting

Converted by the compiler into:

 DoSomething((Color)PrimaryColor.RED); 

If there is an overload, no conversion is made:

  public static void DoSomething(PrimaryColor color)
  {
      Console.WriteLine(color);
  }

DoSomething(PrimaryColor.RED); //Overload is preferred

Enum values if not assigned, continue in a natural order from the last value defined in base enum.

Declare explicit values is recommended, since it can cause a breaking change in the extended enum if you add a property in the base, but everything can work without problems.

I would disallow enum extensions on enum with [Flags] attribute, because even if it could work, it can cause unexpected behaviors and breaking changes if you change values on the base enum.

If you use an underlying type on the base enum, the extended enum will be of the same underlying type .

FaustVX commented 4 years ago

Calling a method should be in the other way.

public void Method(PrimaryColor primary)
{
    // This method expect only RED, GREEN, BLUE and BLACK
}

Method(PrimaryColor.Black); // Works fine
Method(Color.WHITE); // WHITE is an unknown value in Method

But

public void Method(Color primary)
{
    // Method know all Color and PrimaryColor
}

Method(PrimaryColor.Black); // Works fine
Method(Color.WHITE); // Works fine too

So the inheritance works in reverse order.

Flutterish commented 4 years ago

I dont know if I like the "extends" as : already serves this purpose. As @FaustVX said, inheretence would just work in reverse, and I think there would be no issue with this.

As a perk, this feature allows you to

// TODO remove all usage of ObsoleteLibraryEnum, then remove inheritence
enum NewAPIEnum : ObsoleteLibraryEnum { ... }
najak3d commented 2 years ago

I think the absence of this C# feature is costly and fairly prevalent.

The most common solution I keep seeing is the rampant use of naked string values. People just type "{keyName}" and move on, yet they aren't realizing the fragility/weakness of this string-based approach.

But in their defense, what other easy options are available? C# doesn't support what we really need, which is extendible enums.

The process for using Extendible Enums is to simply go to the Custom Enums list, add your name, and then start using it. While you are adding it, Intellisense can be used to find a "Similar/equivalent name" to prevent the addition of names needlessly. And from then on, that name will be encoded, so that you can't commit a typo.

Later if the name is deemed inappropriate and needs to be refactored -- it can now be done easily.

It's a shame that we can't already use Enums for contexts where it's serving more as a "Name-key" that is permitted to grow, or be customized.

I do think the base enum should be required to have an "[ExtendibleEnum]" Attribute, so that users of the enum can be aware that it's designed to have "new members added" (that won't be known by-name, to the assembly that defined the base-enum).

najak3d commented 2 years ago

I have created kludge structs that behave much like Enums, but require hundreds of lines of added code to make them behave acceptably. I term this kludge as "Strongly Typed Strings". Currently I have the following such types:

  1. MapModeName
  2. MapLayerName
  3. MapLayoutName
  4. ShaderID

and so on....

Each of these is just an "int" underneath, but has infrastructure in place to make it work mostly like an Enum value (or a strongly-typed string), and knows it's "string value" and even "Description value". If this were an Enum... the Description would come from an attribute or comment.

I feel pretty clever with this design. But in truth, it's an arduous/inconvenient/inferior kludge -- compared to concept of just having Extendible Enums.

CyrusNajmabadi commented 2 years ago

@najak3d i don't see waht that has to do with this proposal. It soundsl ike you're just discussing Roles/Shapes, which is something we are continuing to look into.

najak3d commented 2 years ago

Please explain what you mean?

How would the Roles/Shapes proposal address the need for an "easy to append to" list of strongly-coded Names? This is what an Enum is -- only the C# enum is non-extendible, so you can't customize it to add new names.

For example, a base assembly may have a dictionary of Items, keyed off of an Enum key. Well, you are sorta stuck with their existing Enum values -- and can't extend them. Unless Enums were extendible.

I see Enums as a better alternative to "strings" - where the string is used as a "Key"... i.e. a typo of the string name will cause the app to "fail". So we want Enums, so that you can't have "typos", and refactoring the name later because easy. While for those using Strings -- you gotta hope you find all instances via string-searches.. And if you mess up -- the app compiles just fine, and you don't see the failure until run-time.

So my concern is for giving C# devs a better built-in replacement for using raw String values as Key values, when those String Values are really restricted to a hard-coded list of values.

So what other proposal provides this, other than the one here?

HaloFour commented 2 years ago

For example, a base assembly may have a dictionary of Items, keyed off of an Enum key. Well, you are sorta stuck with their existing Enum values -- and can't extend them.

IMO, if a base assembly decided to use an enum for that task then they do not intend for you to extend them.

CyrusNajmabadi commented 2 years ago

How would the Roles/Shapes proposal address the need

You're using a mechanism to create strongly typed strings/ints/etc. this is a core case for roles/shapes.

najak3d commented 2 years ago

For example, a base assembly may have a dictionary of Items, keyed off of an Enum key. Well, you are sorta stuck with their existing Enum values -- and can't extend them.

IMO, if a base assembly decided to use an enum for that task then they do not intend for you to extend them.

UNLESS, they mark up their extension with "ExtendibleEnum" in which case they are marking it as an open-ended list of key names. That assembly may only define a few of them. In short they are creating a base type for a list of Strongly-Typed string values... so if they define:

public Dictionary<BaseEnum, CustomDataObject> DataObjects;

Now you can fill this dictionary with data, keyed off of "BaseEnum" values --- and the base class does not care what all values exist, but only cares that you KEY for this dictionary is defined as one of them.

For example, we maintain a Dictionary of MapLayers.. we have to key that off SOMETHING? We can key it off of a raw-string, or we can key it off of a strongly-typed BaseEnum. Now you can't access this Dictionary with anything but a BaseEnum type (or extension of that BaseEnum). It's perfect. Efficient, makes it easy to work with in Intellisense, because it works like an Enum.

I'm only proposing this for Enums marked with the "[ExtendibleEnum]" attribute to make it clear that "these names are not the full list of values that may be used".

najak3d commented 2 years ago

How would the Roles/Shapes proposal address the need

You're using a mechanism to create strongly typed strings/ints/etc. this is a core case for roles/shapes.

Please explain this more. Which specific version of this proposal do you have in mind? And please give me a hint as to how that proposal will enable the creation of a strongly-typed string value, that can be accessed like an enum? (where you are LIMITED to using ONLY the names in the Enum... unless you register a new name, you cannot use it)

Keep in mind that Extendible Enums is about having a "Registered List of Valid Names" all of which appear in Intellisense, as it does for Enums. So you type in the Strongly typed String Name, type "." and boom, there are all of your valid Names to choose from. Just like an Enum... except it can be extended to register new names.

najak3d commented 2 years ago

Right now, I type "MapLayerName." -- and all of my valid MapLayerNames show up in intellisense.

In another assembly, I have another "MapLayerName" that maps to the first one... and so I can add in even more Names, which show up via intellisense.

In this context, the system will not allow you to use a MapLayerName that has not been registered... so you cannot make mistakes. If we don't like the Name, and want to change it -- we just refactor the static member, and it auto-changes all usages of that Name.

This method is inferior to Enums -- but superior to using raw-string values. We have not seen a better way to implement in C#. So far, all we've seen is either people using "enums" or "raw string values".

CyrusNajmabadi commented 2 years ago

Keep in mind that Extendible Enums is about having a "Registered List of Valid Names" all of which appear in Intellisense

You can do this, as i mentioned, with constants or static-readonly's for the well known registered list of names. But you can also decide for your API if you allow people to pass in any name. At which point, users can create their own constants/readonly's for any new values they care about.

CyrusNajmabadi commented 2 years ago

This method is inferior to Enums

I don't see why it's inferior to enums. That seems to be a subjective assessment on your part. Both are simply using another domain (integers, or strings) to give strong names to.

najak3d commented 2 years ago

Statically defined public strings is better than raw-strings.... but in my book, is still a loosely-typed-string. It does not prevent anyone from just typing a String value -- or for having multiple-conflicting sources of static string names. Why? Because it's all loosely-typed strings.

If we can create a "Strongly-Typed String" -- then at least those "Static members" will ALSO be Strongly-Typed Strings -- and so at least then you can search your code for All references of this strongly typed string, and manage it better.

As is -- all are just raw-strings -- without any control of which string you use for your Key.

Is there a proposal to make Strong-Typed-Strings a thing? Where I can create a class that Derives from String? If so -- then I suppose that could be used to reasonably solve this problem.

So if we could declare "static public readonly StrongTypedString" values, then Yes, this will resolve the main problem here.

(Because the readonly trait on these static strings should put them into a place that doesn't burden the GC, right?)

najak3d commented 2 years ago

This method is inferior to Enums

I don't see why it's inferior to enums. That seems to be a subjective assessment on your part. Both are simply using another domain (integers, or strings) to give strong names to.

We wrapped ours with int's, because our MapLayerName is a "struct" ... and so doesn't bother the heap or GC. Very cheap, like a flyweight. It's index references back to a static array of Names.

We could have wrapped a string, but now we are classes, not structs, which has drawbacks.

CyrusNajmabadi commented 2 years ago

Statically defined public strings is better than raw-strings.... but in my book, is still a loosely-typed-string. It does not prevent anyone from just typing a String value

I mean, that is true of enums as well. It's just a wrapper around an integral. Anyone can pass in any value from the integral domain of that enum.

OrbintSoft commented 2 years ago

To be honest I don't understand the need of using raw strings. enums in fact are structs that extends numbers, if you want to extend them you just need to create a new enum, duplicate the values and do an explicit casting

Even if you rename a value is not a problem, but you can have problems if you change the numeric value, or if you add a new value to the base enum. For example if I define PrimaryColor.INDIGO=8000ff and I forget to define in Color enum. When I try to do myMethod((Color)PrimaryColor.INDIGO) for the compiler everything is fine, but casting will fails at runtime.

My proposal is to create a syntax that automatically copies the value of base enum into extended enum, and converts automatically this code myMethod(PrimaryColor.INDIGO) to this myMethod((Color)PrimaryColor.INDIGO) when my method is defined as :

public void Method(Color primary)
{
    // Method know all Color and PrimaryColor
}

This is not an object inheritance and has nothing to do with that, that's why I proposed a new keyword instead of :

CyrusNajmabadi commented 2 years ago

Keep in mind that Extendible Enums is about having a "Registered List of Valid Names" all of which appear in Intellisense,

There is no need to augment the language just for tooling. You can do tooling with that. Note that we already support this concept somewhat in that a type can state another type that you should use to get values of it from. e.g. if something takes a Color type, it can have a doc comment marker on it saying that the user should be shown items from a Colors class that contains instance of hte Color type. We could always expand on that to let others add in this scenario.

najak3d commented 2 years ago

I want type-safety, which you have with Enums. I can't send in an "int" or "string" when it asks for an Enum type.. thus I'm forced to write good key values. If the accepting function takes raw strings, we lose out on safety.

A strong-typed string would be nice, but I'm guessing that's not on-the-table for implementation.

If I could make a strong-typed String (derive directly from string) - it would make it very easy to solve the problem I'm trying to solve. So either that, or Extendible Enums.

najak3d commented 2 years ago

Other thing I can do with Enum is "Find all References", while for string value -- not so easy. Enums are very awesome; exactly what I want to use -- if only they could be extended.

CyrusNajmabadi commented 2 years ago

I can't send in an "int" or "string" when it asks for an Enum type..

Yes you can :)

Enums don't give typesafety at all. They're similar to NRT in that it's a general suggestion which the callee doesn't have to respect at all. Indeed, it's normal for APIs to use enums with a public surfce area that is documented, and an internal area that is undoc'ed, but which still works as the caller can pass whatever they want.

CyrusNajmabadi commented 2 years ago

Other thing I can do with Enum is "Find all References", while for string value -- not so easy

We support Find-All-References in string-values as well :)

But, again, that's a tooling concern, not the language concern.

najak3d commented 2 years ago

There can be multiple instances of a string, that are NOT the Key. So in having strings, you risk the chance of accidently mistaking a matching string value for one that is being used as this strong-typed key name. And if you happen to forget one, while refactoring, then it'll remain unfound/orphaned, yet will compile fine -- but then will fail at some point run-time.

This is 100% a language concern. There is no amount of tooling that could fix the raw-string conundrum. We either need strongly-typed strings, or Extendible Enums, to properly solve this issue best. (Or another base struct, that behaves exactly like an enum, except that it's extendible outside of the assembly that declares the base enum.)

HaloFour commented 2 years ago

Don't use strings as keys if you don't want people to use strings as keys. If you want a strongly-typed value, declare a strong type and control how instances are created. The language doesn't need to provide a specific solution to a problem, only a solution, and there exist many ways to solve this problem.

CyrusNajmabadi commented 2 years ago

There can be multiple instances of a string, that are NOT the Key.

This is hte same with an enum as well. If you want to prevent this you can use an analyzer. But you'll also need to back it up with a runtime check.

CyrusNajmabadi commented 2 years ago

We either need strongly-typed strings, or Extendible Enums

Once you have 'extendible enums' your'e entirely saying: the values here are open ended (which is already true of enums and strings anyways). So you're literally saying: any values can be passed in, so nothing would protect you here anyways.

najak3d commented 2 years ago

Don't use strings as keys if you don't want people to use strings as keys. If you want a strongly-typed value, declare a strong type and control how instances are created. The language doesn't need to provide a specific solution to a problem, only a solution, and there exist many ways to solve this problem.

The "many ways to solve this problem" are costly. For most C# language changes, there were "many ways to solve" those problems too, but the cost of solving it otherwise was burdensome or problematic. So C# was adapted to make it easy.

The exact type I WANT to use for many key named contexts (where the key name is hard coded inside C#) is an ENUM. It's the most fitting for the situation by far.... except for it's inability to be extended, which then later can bite you, because another user/assembly cannot add values when they need to.

And so what do programmers do instead? Use Strings, because it's the lesser of evils, given that it's a POTENTIAL REQUIREMENT to enable extensibility later. They stop using ENUMS and start using Strings for nearly all Keys, where there MIGHT be a chance that they want to extend the Enum Values later. They do this because they "got bit" by Enum's deficiency of non-extensibility... and therefore avoid them.

Enums are uber-powerful as a Strong-Typed-String equivalent, limiting names to only the registered names. It's an 'int' underneath so super powerful, and when used as a Dictionary-key, works faster than a string.

I'd LOVE to use them, but I can't, because of the POTENTIAL requirement that the Enum Names needs to be extended by a customer. And so we are forced into doing a kludge/burdensome (and inferior) implementation of an Enum type.

I have to include about 200 compact lines of added C# to create this struct that is near equivalent to an Enum Type, which I'd like to write just once as a template -- but this isn't possible because you can't derive from structs, and the private static Names List would then be shared by ALL derivations of this struct -- so that would be problematic too.

MOST C# programmers that I've seen/met don't do what I've done; but instead just use Strings, because it's the "Path of least resistance" without really comprehending the QA risks they've just introduced.

If C# had Extendable Enums -- programmers would naturally again use Enums for these situations, and all would be well again in this Universe.

jmarolf commented 2 years ago

Yeah I don't really understand what problem we are trying to solve here. This sort of reminds me of subtypes in ada

subtype Rainbow   is Color range Red .. Blue;
subtype Red_Blue  is Rainbow;
subtype Int       is Integer;
subtype Small_Int is Integer range -10 .. 10;
subtype Up_To_K   is Column range 1 .. K; 
subtype Square    is Matrix(1 .. 10, 1 .. 10);
subtype Male      is Person(Sex => M);
subtype Binop_Ref is not null Binop_Ptr;

where you could say something like (I dunno making up some C# syntax here)

public enum PrimaryColor subtype Color { RED, GREEN, BLUE, BLACK }

or perhaps for general types (something similar to this use case) you could do:

public class OSPlatform subtype string { "Free BSD", "Linux", "Windows" }

Essentially I think the more common case is wanting to restrict an existing types which roles seems to be current design we are working toward.

Is the widening of an enum just because enums are classes in languages like Java and folks want to do inheritance things to them? I might be able to be convinced that there is some utility in enum inheritance, but it certainly doesn't make apis less error prone. Today enums outside of the defined values are rare and awkward because you need to do an explicit cast. If enums are allowed to have inheritance chains that add new values, you now need to worry about someone passing you a derived enum with value ranges you do not expect.

najak3d commented 2 years ago

Here's some sample code that I wrote to get around the Enum deficiency, and create something that approximates an Enum:

(there's a bit of added code, since we were using a struct, I added a little more smarts into it, so that each enum was also aware of it's 'group')

```c# public partial struct MapModeName : IComparable, IComparable { static public MapModeName InstrumentsPanel; static public MapModeName ProcedurePreview; // old: Preview static public MapModeName FullScreenPlate; // old: NonGeorefPlates static public MapModeName EFIS; static public MapModeName Vector; static public MapModeName VFR; static public MapModeName LowIFR; static public MapModeName HighIFR; static public MapModeName Custom; static private void ___LoadDefaults() { InstrumentsPanel = __Create("InstrumentsPanel", "Instruments Panel", _GROUPS.InstrumentsPanel); ProcedurePreview = __Create("ProcedurePreview", "Procedure Preview", _GROUPS.ProcedurePreview); FullScreenPlate = __Create("FullScreenPlate", "Full Screen Plate", _GROUPS.FullScreenPlate); EFIS = __Create("EFIS", "EFIS", _GROUPS.EFIS); Vector = __Create("Vector", "Vector", _GROUPS.VectorMap, true); VFR = __Create("VFR", "VFR", _GROUPS.RasterMap, true); LowIFR = __Create("LowIFR", "LowIFR", _GROUPS.RasterMap, true); HighIFR = __Create("HighIFR", "HighIFR", _GROUPS.RasterMap, true); Custom = __Create("Custom", "Custom", _GROUPS.CustomMap, true); _DEFAULT = Vector; } static public void __LoadDefaults() { if (!__UseDefaults) return; lock (s_AllNameLookup) { if (s_AreDefaultsLoaded) return; // already loaded! s_AreDefaultsLoaded = true; _GROUPS.__LoadDefaults(); ___LoadDefaults(); } } static public bool __UseDefaults { get { return s_UseDefaults; } set { if (s_UseDefaults == value) return; // no change if (s_AreDefaultsLoaded) { Log.CodingError("MapLayerName.UseDefaults.Set() - called after Defaults already loaded."); } s_UseDefaults = value; } } static private bool s_UseDefaults = true; static private bool s_AreDefaultsLoaded; private class JsonConverter : J.JsonConverter { public override MapModeName ReadJson(J.JsonReader reader, Type objectType, MapModeName existingValue, bool hasExistingValue, J.JsonSerializer serializer) { string name = (string)reader.Value; return MapModeName.__GetByName(name); } public override void WriteJson(J.JsonWriter writer, MapModeName value, J.JsonSerializer serializer) { writer.WriteValue(value.Name); } } static public readonly MapModeName _NULL; static public MapModeName _DEFAULT; static MapModeName() { _NULL = __Create("_NULL_", "NullMapMode", "_NULL_"); } public readonly byte Index; public readonly byte GroupIndex; public readonly bool IsEFISMode; public readonly bool IsNormalMap; public string Name { get { return __AllInstanceNames[Index]; } } public string DisplayName { get { return __AllInstanceDisplayNames[Index]; } } public bool IsNull { get { return Index == 0; } } public bool IsValid { get { return Index > 0; } } public bool IsMapMode { get { return !IsEFISMode && IsValid; } } public string GroupName { get { return _GROUPS.__GetGroupName(GroupIndex); } } public string GroupDisplayName { get { return _GROUPS.__GetGroupDisplayName(GroupIndex); } } public override string ToString() { return Name; // "MapMode<" + Name + "> = " + DisplayName; } static public MapModeName __Create(string nameOfMode, string displayName, string groupName, bool isNormalMap = false) { byte groupIndex = _GROUPS.__GetOrCreateGroupName(groupName); return __Create(nameOfMode, displayName, groupIndex, isNormalMap); } static public MapModeName __Create(string nameOfMode, string displayName, byte groupIndex = 0, bool isNormalMap = false) { MapModeName modeName; lock (s_AllNameLookup) { isNormalMap = isNormalMap; if (!s_AllNameLookup.TryGetValue(nameOfMode, out modeName)) { // Not found - so create -- this is EXPECTED if (__InstanceCount >= __MAX_NAMES) { Log.CodingError("MapModeName.Create() - too many created!: {0}, {1}, {2}", __InstanceCount, modeName, displayName); __InstanceCount--; } bool isEFIS = nameOfMode.Contains("EFIS"); modeName = new MapModeName(__InstanceCount, groupIndex, isEFIS, isNormalMap); __InstanceCount++; } else { } // already exists -- so we're going to overwrite it int index = modeName.Index; __AllInstances[index] = modeName; __AllInstanceNames[index] = nameOfMode; __AllInstanceDisplayNames[index] = displayName; s_AllNameLookup[nameOfMode] = modeName; } return modeName; } private MapModeName(byte index, byte groupIndex, bool isEFISMode, bool isNormalMap) { Index = index; GroupIndex = groupIndex; IsEFISMode = isEFISMode; IsNormalMap = isNormalMap; } static public MapModeName __GetByIndex(byte index) { return __AllInstances[index]; } static public MapModeName __GetByName(string nameOfMode) { if (string.IsNullOrEmpty(nameOfMode)) return MapModeName._NULL; MapModeName modeName; if (!s_AllNameLookup.TryGetValue(nameOfMode, out modeName)) { modeName = MapModeName._NULL; } return modeName; } public const byte __MAX_NAMES = 255; static private Dictionary s_AllNameLookup = new Dictionary(__MAX_NAMES); static private string[] __AllInstanceNames = new string[__MAX_NAMES]; static private string[] __AllInstanceDisplayNames = new string[__MAX_NAMES]; static private MapModeName[] __AllInstances = new MapModeName[__MAX_NAMES]; static public byte __InstanceCount { get; private set; } static public IEnumerable __GetAllInstances() { for (byte i = 0; i < __InstanceCount; i++) yield return __AllInstances[i]; } public override int GetHashCode() { return Index; } public override bool Equals(object obj) { if (obj == null || !(obj is MapModeName)) { return false; } MapModeName other = (MapModeName)obj; return Index == other.Index; } int IComparable.CompareTo(MapModeName other) { return Index.CompareTo(other.Index); } int IComparable.CompareTo(object obj) { if (obj is MapModeName) { MapModeName other = (MapModeName)obj; return Index.CompareTo(other.Index); } throw new ArgumentException("MapModeName.CompareTo() with wrong object type."); } static public bool operator ==(MapModeName left, MapModeName right) { return left.Index == right.Index; } static public bool operator !=(MapModeName left, MapModeName right) { return left.Index != right.Index; } static public bool operator <(MapModeName left, MapModeName right) { return left.Index < right.Index; } static public bool operator >(MapModeName left, MapModeName right) { return left.Index > right.Index; } static public bool operator <=(MapModeName left, MapModeName right) { return left.Index <= right.Index; } static public bool operator >=(MapModeName left, MapModeName right) { return left.Index >= right.Index; } } ```
najak3d commented 2 years ago

The equivalent of my code above (almost) if it were just an enum is this:

public enum MapModeName 
{ 
    _NULL, InstrumentPanel, ProcedurePreview, FullScreenPlate, EFIS, Vector, VFR, LowIFR, HighIFR, Custom
}
najak3d commented 2 years ago

This is why I'm here, asking for the reasons not to just make Enums with the "[ExtendibleEnum]" attribute, extendible.

najak3d commented 2 years ago

@jmarolf - This should only be enabled for Enums with the "[EnumExtendible]" attribute, so that it doesn't open up any cans of worms. It simply allows the cases where the enum should be extended, to be extended. Then it just serves as a strong-typed list of registered string names, effectively.

CyrusNajmabadi commented 2 years ago

@jmarolf - This should only be enabled for Enums with the "[EnumExtendible]" attribute, so that it doesn't open up any cans of worms. It simply allows the cases where the enum should be extended, to be extended. Then it just serves as a strong-typed list of registered string names, effectively.

But it's not strongly typed. I've mentioned this a few times :) Enums are not strongly typed. They must have runtime validation as your clients have no restriction on passing any value they want to it :)

najak3d commented 2 years ago

@jmarolf - This should only be enabled for Enums with the "[EnumExtendible]" attribute, so that it doesn't open up any cans of worms. It simply allows the cases where the enum should be extended, to be extended. Then it just serves as a strong-typed list of registered string names, effectively.

But it's not strongly typed. I've mentioned this a few times :) Enums are not strongly typed. They must have runtime validation as your clients have no restriction on passing any value they want to it :)

@Cyrus: Fine, then call Enums "Specifically Typed", where as a "string" value is very non-specifically typed. Enums imply meaning by the name of the enum itself. While for a string value -- it is tied to nothing specific, and opens the door to QA issues.

The goal is clarity, QA, and efficiency. The Enum is superior to strings in all of these areas. The only thing it's lacking is the ability to be Extended with the presence of a very clear '[ExtendibleEnum]' attribute.

Sad use of strings in C# code is rampant, where enums would have been a better choice -- if only the enum were extendable.

CyrusNajmabadi commented 2 years ago

if only the enum were extendable.

Enums are already extendible. The entire domain of their integral backing types is available. It's technically finite. But so large that it's effectively infinitely large (just as strings).

CyrusNajmabadi commented 2 years ago

@cyrus: Fine, then call Enums "Specifically Typed", where as a "string" value is very non-specifically typed.

I don't know what the distinction would be here. What do you mean by 'specifically typed'?

najak3d commented 2 years ago

@CyrusNajmabadi - it's like you are trolling on your own site. Should I report you?

Specifically-typed means that "enum MapModeName" is clear that all of it's members are Map Mode Names ... so a value of "Vector" means "Map Mode Name = Vector". While the loose string value of "Vector" doesn't indicate that it's a "Map Mode Name", and so it's preferable to send a parameter "MapModeName.Vector" vs. just sending a raw string value "Vector".

You didn't like the term "Strongly Typed" since typing isn't strictly enforced. So I suppose "Specifically typed" is a better term, since this is about specificity/clarity and easy-QA/Intellisense. I'm not trying to claim you can't caste another int-type to an enum-type... that becomes a "don't care" since we're not trying to say Enums are hack-proof.

CyrusNajmabadi commented 2 years ago

Specifically-typed means that "enum MapModeName" is clear that all of it's members are Map Mode Names

But how can you state anything about all of it's members when:

  1. a client can pass any other value without problem.
  2. you want to open this to extension, meaning that you don't know the set of known members up front?
CyrusNajmabadi commented 2 years ago

and easy-QA/Intellisense

This is not hte domain of hte language though. Intellisense can already be adjusted through things like changing roslyn.

since we're not trying to say Enums are hack-proof.

So it's still unclear waht value is being gained here. It sounds like you both want well known names up front, but also want people to be able to add names later. That can be done today on your own just with things like strings/enums and an analyzer on your part. As you say, you don't care about hack-proof. So thsi approach is more than sufficient to meet your objectives.

HaloFour commented 2 years ago

@najak3d

Sounds kinda like you want Java enums, which are patented.

CyrusNajmabadi commented 2 years ago

@HaloFour can java enums be extended after the fact by a different party?

HaloFour commented 2 years ago

@CyrusNajmabadi

can java enums be extended after the fact by a different party?

That they can't. The closest you'd get is to have the key be of an interface type, and the enum can implement that interface, as could another enum. But given the blob of code that was posted above, that reminds me very much of Java enums.

DUs might also fit that bill, if they can implement interfaces.

najak3d commented 2 years ago

Something equivalent to Java Enums would be nice. It would be nice if C# just supported something like that as a native type, where under the covers, it's just a wrapped-int (index), and so operates like a fly-weight -- you are just passing around an int... but that int is the index into a table/array of additional data, such as "Name" for starters, and other data if it helps (this is how we've implemented it in our own struct). But as you can see, in C# generating this functionality requires about 200 lines of added code per Extendible-Enum, which is why it's never used.

Instead, most C# dev seem to flock towards use of String Keys. Most devs aren't aware that there's a way emulate a Java-like enum using a C# structure (as I've done), and are intrigued when they see it. But as I implement these specialized/tedious structs, it begs the question - why aren't select C# enum types simply extendable?

Most don't even ask the question -- they just use strings, and have become used to the added risks to QA -- so when bugs happen as a result, they get paid more hours to fix those bugs. No sweat off their back.

najak3d commented 2 years ago

It would also be less painful, if the pattern that I implemented for MapLayerName can be "re-used" other than cut-paste of the actual code. But since it's a struct and makes use of static private Lists/Arrays/Tables - it wouldn't work out to have a shared base class among many derivations -- as they all be sharing the same static arrays/table -- which is a stopper.

If there was a more elegant way of "writing this implementation once" and then re-using it, so that derivations were very light weight (e.g. all they had to do was define their static member list, and be done) -- then that would make this more appealing, and might help C# developers to stop using Strings Key names so rampantly.

najak3d commented 2 years ago

Without implementing it on each Enum-struct, is there a way to add the following methods without duplicating this code inside of each struct-Enum that I make?

If so - that too would make writing your own struct-Enums less ~20% less tedious.

```c# public override int GetHashCode() { return Index; } public override bool Equals(object obj) { if (obj == null || !(obj is MapLayerName)) { return false; } MapLayerName other = (MapLayerName)obj; return Index == other.Index; } int IComparable.CompareTo(MapLayerName other) { return Index.CompareTo(other.Index); } int IComparable.CompareTo(object obj) { if (obj is MapLayerName) { MapLayerName other = (MapLayerName)obj; return Index.CompareTo(other.Index); } throw new ArgumentException("MapLayerName.CompareTo() with wrong object type."); } static public bool operator ==(MapLayerName left, MapLayerName right) { return left.Index == right.Index; } static public bool operator !=(MapLayerName left, MapLayerName right) { return left.Index != right.Index; } ```

I'm already devising a way to consolidate about half of the remaining code into a re-usable "StructEnumData" class, which manages the data lists/tables for each type, thus making it less tedious.

I'd really like to stop seeing so many string-based Key names in C# code. Until there is a viable and well-known alternative, this'll likely continue to be the trend.

CyrusNajmabadi commented 2 years ago

You can do this with a source generator.

CyrusNajmabadi commented 2 years ago

and might help C# developers to stop using Strings Key names so rampantly.

You haven't identified any problems going that route, so it's unclear why people would stop.

najak3d commented 2 years ago

and might help C# developers to stop using Strings Key names so rampantly.

You haven't identified any problems going that route, so it's unclear why people would stop.

I suppose then you are one of those who uses string-based keys? Most people who use them have been doing it for so long that they don't see the associated QA issues, and since they know there are no other viable options (that are extendable), they just use them an move on, being that this is the best option in C# now available (that they know of). If so, then I may barking to deaf ears.

It's hard for me to believe that you don't see the inferiority of using "string based keys" vs. an Extendible-Enum of names. Strings are non-specifically typed and can be mistyped easily, while Enum values cannot be mistakenly mistyped without a compiler error... and can't be missed on refactoring a Name without causing compiler errors.

I've said this many times -- but you still don't see the gains in what I am saying. This is a bit disappointing, given that you are one of the guardians of this great language.

najak3d commented 2 years ago

All of my griping has lead me to innovate an improvement on my current struct-based solution. I have now added an interface call IStructEnum, and a new helper class called StructEnumHelper. Both of these reduce the amount of cut-pasted code to less than half of what it was, and most of what's left is non-tedious.

Here's the resulting interface and helper class:

```c# public interface IStructEnum { int Index { get; } string Name { get; } } public class StructEnumHelper where T : IStructEnum { public class JsonConverter : J.JsonConverter { public override T ReadJson(J.JsonReader reader, Type objectType, T existingValue, bool hasExistingValue, J.JsonSerializer serializer) { string name = (string)reader.Value; return Instance.GetByName(name); } public override void WriteJson(J.JsonWriter writer, T value, J.JsonSerializer serializer) { writer.WriteValue(value.Name); } } static public StructEnumHelper Instance { get; private set; } public int Capacity { get; private set; } public T[] __AllInstances; public Dictionary s_AllNameLookup; public string[] __AllInstanceNames; public string[] __AllInstanceDisplayNames; public byte __InstanceCount { get; private set; } public T NullValue { get; private set; } public T UnknownValue { get; private set; } public readonly string Name; public StructEnumHelper(int maxEntries) { Instance = this; Capacity = maxEntries; Name = typeof(T).Name; __AllInstances = new T[Capacity]; __AllInstanceNames = new string[Capacity]; __AllInstanceDisplayNames = new string[Capacity]; s_AllNameLookup = new Dictionary(Capacity); } public T AddNew(string name, string displayName, Func constructor, Action onAdded) { T newItem; lock (s_AllNameLookup) { if (!s_AllNameLookup.TryGetValue(name, out newItem)) { // Not found - so create -- this is EXPECTED if (__InstanceCount >= Capacity) { Log.CodingError("MapLayerName.Create() - too many created!: {0}, {1}, {2}", __InstanceCount, newItem, displayName); __InstanceCount--; } newItem = constructor(); // new MapLayerName(__InstanceCount, index2, isEnabledByDefault); if (__InstanceCount == 0) NullValue = newItem; if (__InstanceCount == 1) UnknownValue = newItem; __InstanceCount++; } else { } // already exists -- so we're going to overwrite it int index = newItem.Index; __AllInstances[index] = newItem; __AllInstanceNames[index] = name; __AllInstanceDisplayNames[index] = displayName; s_AllNameLookup[name] = newItem; onAdded?.Invoke(newItem); } return newItem; } public IEnumerable GetAllInstances() { for (byte i = 0; i < __InstanceCount; i++) yield return __AllInstances[i]; } public T GetByIndex(byte index) { return __AllInstances[index]; } public void SetInstance(T enumVal) { __AllInstances[enumVal.Index] = enumVal; } public T GetByName(string nameOfLayer) { if (string.IsNullOrEmpty(nameOfLayer)) return NullValue; T layerName; if (!s_AllNameLookup.TryGetValue(nameOfLayer, out layerName)) { layerName = UnknownValue; } return layerName; } public int GetHashCode(T enumVal) { return enumVal.Index; } public int CompareTo(T enumVal, T other) { return enumVal.Index.CompareTo(other.Index); } public int CompareTo(T enumVal, object obj) { if (obj is T other) { return enumVal.Index.CompareTo(other.Index); } throw new ArgumentException($"StructEnumHelper.CompareTo() with wrong object type: {Name}"); } public bool IsEqual(T enumVal, object obj) { return (obj is T other) ? enumVal.Index == other.Index : false; } public bool IsEqual(T left, T right) { return left.Index == right.Index; } public bool IsNotEqual(T left, T right) { return left.Index != right.Index; } } ```

And now one struct that now uses this helper is here -- this code is almost the same among structEnums -- all I have to do is cut/paste the code and change "MapLayerName" to the new enum name. Easy.

[J.JsonConverter(typeof(StructEnumHelper<MapLayerName>.JsonConverter))]
public partial struct MapLayerName : IComparable, IComparable<MapLayerName>, IStructEnum
{
  int IStructEnum.Index => Index;

  public const byte __MAX_NAMES = 255;
  static private StructEnumHelper<MapLayerName> s_Helper = new StructEnumHelper<MapLayerName>(__MAX_NAMES);
  static public readonly MapLayerName NULL = new MapLayerName();

  public readonly byte Index;

  public string Name => s_Helper.__AllInstanceNames[Index];
  public string DisplayName => s_Helper.__AllInstanceDisplayNames[Index];

  public bool IsNull => Index == 0;
  public bool IsValid => Index > 0;
  public override string ToString() => $"{s_Helper.Name}<{Name}> = {DisplayName}";
  static public MapLayerName __GetByIndex(byte index) => s_Helper.GetByIndex(index);
  static public MapLayerName __GetByName(string name) => s_Helper.GetByName(name);
  static public int __InstanceCount => s_Helper.__InstanceCount;
  public override int GetHashCode() => s_Helper.GetHashCode(this);
  public override bool Equals(object obj) => s_Helper.IsEqual(this, obj);
  int IComparable<MapLayerName>.CompareTo(MapLayerName other) => s_Helper.CompareTo(this, other);
  int IComparable.CompareTo(object obj) => s_Helper.CompareTo(this, obj);
  static public bool operator ==(MapLayerName left, MapLayerName right) => s_Helper.IsEqual(left, right);
  static public bool operator !=(MapLayerName left, MapLayerName right) => s_Helper.IsNotEqual(left, right);

  static public MapLayerName __Create(string name, string displayName, byte index2 = 0, bool isEnabledByDefault = true)
  {
      MapLayerName layerName = s_Helper.AddNew(name, displayName,
          () => new MapLayerName(s_Helper.__InstanceCount, index2, isEnabledByDefault),
          (item) => { __RenderOrderMap[item.Index] = RenderOrder.__AssociateMapLayer(item); }
          );
      return layerName;
  }
   // the rest is custom but not much left to write, except for extra data I add specific to this type....
}
najak3d commented 2 years ago

All of this.. to create a type that still has some inferiorities to Enums. For example if you type:

 myLayerName = 

There is no Intellisense help to bring up the "MapLayerName.", so you have to start typing it... where as for Enum types, the intellisense auto-shows the "MapLayerName.{options}" for you.

Before you say "well that's a tooling issue" -- that's a silly thing to say. The goal here isn't to "make things nice for JUST ME", but rather to address the fact that "look at all this code it takes to create an extendible StructEnum"!! This is why people just use String-Keys, because nobody knows about this option.

Extendable Enums -- would be superior to what I have done, and would become used by the mainstream, and would replace the current trends of using String Keys.

While as for my solution - almost nobody will ever find out about it, nor discover it themselves. They'll continue using string keys...