ardalis / SmartEnum

A base class for quickly and easily creating strongly typed enum replacements in C#.
MIT License
2.15k stars 170 forks source link

JsonConverter doesn't support string Value type #111

Open mikesigs opened 3 years ago

mikesigs commented 3 years ago

I have a usecase where we need to deal with string values as the backing value for an enumeration. As an example, imagine a Customer Type stored in the database as a string code:

SmartEnum can handle that if you define your type as SmartEnum<CustomerTypeCode, string>. However the associated SmartEnum.JsonNet package doesn't allow this because it constraints the TValue to be a struct, which string is not.

Example:

public class CustomerTypeCode: SmartEnum<CustomerType, string>
{
    public static readonly CustomerTypeCode Regular = new CustomerTypeCode(nameof(Regular), "reg");
    public static readonly CustomerTypeCode Preferred = new CustomerTypeCode(nameof(Preferred), "pref");
    public static readonly CustomerTypeCode Elite = new CustomerTypeCode(nameof(Elite), "el");

    public DecisionResponseCode(string name, string value) : base(name, value) { }
}

public class Customer
{
    // ERROR: The type 'string' must be non-nullable value type in order to use it as parameter 'TValue' ...
    [JsonConverter(typeof(SmartEnumValueConverter<CustomerTypeCode, string>))]
    public CustomerTypeCode Code { get; set; }
}

I get a compiler error when trying to use either SmartEnumValueConverter<CustomerTypeCode, string> or SmartEnumNameConverter<CustomerTypeCode, string> because the generic type constraints of TValue include struct, whereas TValue in SmartEnum doesn't have this constraint.

public abstract class SmartEnum<TEnum, TValue> 
    : IEquatable<SmartEnum<TEnum, TValue>>, 
      IComparable<SmartEnum<TEnum, TValue>> 
      where TEnum : SmartEnum<TEnum, TValue> 
      where TValue : IEquatable<TValue>, IComparable<TValue>
public class SmartEnumValueConverter<TEnum, TValue> 
    : JsonConverter<TEnum> 
      where TEnum : SmartEnum<TEnum, TValue> 
      where TValue : struct, IEquatable<TValue>, IComparable<TValue>

Is there any reason to not use string as your backing value, that this type constraint is in place? Or can the restriction be removed in a future update? For now I can write my own type converter, but I figured I'd submit this as a bug (in case it is one).

Thanks!

ardalis commented 3 years ago

Other converters had the constraint to be structs until very recently; I think it would probably be fine to remove the constraint here as well. At least, I'm not aware of any issues it would introduce.

jafin commented 3 years ago

@mikesigs does #106 fix this? I believe it was merged but will need a new build of the SmartEnum.JsonNet lib ? This passes for me when run on current master branch, but I don't think the code has been pushed to a new version on nuget for the SmartEnum.JsonNet assembly.

public class CustomerTypeCode : SmartEnum<CustomerTypeCode, string>
        {
            public static readonly CustomerTypeCode Regular = new CustomerTypeCode(nameof(Regular), "reg");
            public static readonly CustomerTypeCode Preferred = new CustomerTypeCode(nameof(Preferred), "pref");
            public static readonly CustomerTypeCode Elite = new CustomerTypeCode(nameof(Elite), "el");

            public CustomerTypeCode(string name, string value) : base(name, value) { }
        }

        public class Customer
        {
            // ERROR: The type 'string' must be non-nullable value type in order to use it as parameter 'TValue' ...
            [JsonConverter(typeof(SmartEnumValueConverter<CustomerTypeCode, string>))]
            public CustomerTypeCode Code { get; set; }
        }

        [Fact]
        public void Test()
        {
            var x = new Customer(){Code = CustomerTypeCode.Elite};
            var json = JsonConvert.SerializeObject(x);
            json.Should().NotBeEmpty();
        }
mikesigs commented 3 years ago

@jafin That's good to hear. Looks like the last release of SmartEnum.JsonNet was just over a year ago. Would love to see a new release soon.

jafin commented 3 years ago

@mikesigs I think i saw this got in a release 1.0.31 https://github.com/ardalis/SmartEnum/releases/tag/SmartEnumJsonNet-v1.0.31

mikesigs commented 3 years ago

Nice I will check it out!