Validating list of values to be sent to API

Question

I'm currently writing a client library to use with a courier's SOAP API to create and manage shipments. I'm using the service classes generated from the WSDL provided by the courier, their implementation is on the verbose side (multiple objects nested inside each other to specify a country code for an address as an example), so I'm trying to write a client library I can reuse that uses a cleaner set of classes.

I'm trying to have the library perform some basic validation on data so it can catch errors before it gets sent to the API. When creating a shipment there are several options that need to be supplied to specify the service type/format and other enchantments, the API requires a string code (1 to 3 characters) and they supply a list of codes and what they each mean (For example P = Package, L = Letter for shipment format). To implement these options in my library I decided to use the type-safe-enum pattern to provide any consumers of my library an easy way of specifying values, and allowing my library a way of checking the options without having to use magic values when checking for validation, for example the Email Notification enhancement would require a email be provided for the API request to be accepted by the courier's system, so instead of checking the enhancements list for "14" I would check for ServiceEnhancement.EmailUpdates making the code easier to understand. A consumer might also want to get one of these options by code and I want my library to thrown an exception if an invalid code is given.

This all seems to work but I now have 6 classes all with almost exactly the same code (only the Class name and the list of enum variables change) which says to me I should be using inheritance, but I'm fairly sure I can't, due to the fact the static GetByCode function's return type needs to match the class its in. I did look into using generics but I'm not sure they are right for this use case.

public sealed class ServiceEnhancement
{
    // This dictionary must come before the public static fields, otherwise it won't be declared in time.
    // See: https://stackoverflow.com/a/424414/2920332
    private static readonly Dictionary<string, ServiceEnhancement> Instance =
        new Dictionary<string, ServiceEnhancement>();

    public static ServiceEnhancement Loss1000 = new ServiceEnhancement("1", "Consequential Loss £1000");
    public static ServiceEnhancement Loss2500 = new ServiceEnhancement("2", "Consequential Loss £2500");
    public static ServiceEnhancement Loss5000 = new ServiceEnhancement("3", "Consequential Loss £5000");
    public static ServiceEnhancement Loss7500 = new ServiceEnhancement("4", "Consequential Loss £7500");
    public static ServiceEnhancement Loss10000 = new ServiceEnhancement("5", "Consequential Loss £10000");
    public static ServiceEnhancement Recorded = new ServiceEnhancement("6", "Recorded");
    public static ServiceEnhancement Loss750 = new ServiceEnhancement("11", "Consequential Loss £750");
    public static ServiceEnhancement TrackedSignature = new ServiceEnhancement("12", "Tracked Signature");
    public static ServiceEnhancement SmsUpdates = new ServiceEnhancement("13", "SMS Notification");
    public static ServiceEnhancement EmailUpdates = new ServiceEnhancement("14", "E-Mail Notification");
    public static ServiceEnhancement SmsAndEmailUpdates = new ServiceEnhancement("16", "SMS + E-Mail Notification");
    public static ServiceEnhancement LocalCollect = new ServiceEnhancement("22", "Local Collect");
    public static ServiceEnhancement SaturdayGuaranteed = new ServiceEnhancement("24", "Saturday Guaranteed");

    private readonly string _code;
    private readonly string _name;

    private ServiceEnhancement(string code, string name)
    {
        _code = code;
        _name = name;
        Instance.Add(code, this);
    }

    public static ServiceEnhancement GetByCode(string code)
    {
        if (!Instance.ContainsKey(code))
        {
            throw new ArgumentException($"Code '{code}' not a valid ServiceEnhancement.", "code");
        }

        return Instance[code];
    }

    public string GetName()
    {
        return _name;
    }

    public override string ToString()
    {
        return _code;
    }
}

I'm thinking I might be better off dropping the whole enum aspect and just going with a List/Dictionary to handle the validation and just comment the usage of magic values inside my library, for example:

    private bool NeedsMobileNumber => ServiceEnhancements.Any(se =>
        se.Equals(ServiceEnhancement.SmsUpdates) || se.Equals(ServiceEnhancement.SmsAndEmailUpdates));

    private bool NeedsEmailAddress => ServiceEnhancements.Any(se =>
        se.Equals(ServiceEnhancement.EmailUpdates) || se.Equals(ServiceEnhancement.SmsAndEmailUpdates)); 

Would become:

    // Check for SMS Notification Enhancement (13) and SMS + E-Mail Notification Enhancement (16)
    private bool NeedsMobileNumber => ServiceEnhancements.Any(se =>
        se.Equals("13") || se.Equals("16"));

    // Check for Email Notification Enhancement (13) and SMS + E-Mail Notification Enhancement (16)
    private bool NeedsEmailAddress => ServiceEnhancements.Any(se =>
        se.Equals("14") || se.Equals("16"));

Answer 1

I'm thinking I might be better off dropping the whole enum aspect

No, no, no, don't you dare!

I guess you are the first person on Code Review asking for advice about how to better worsen the code ;-)

private bool NeedsMobileNumber => ServiceEnhancements.Any(se =>
    se.Equals(ServiceEnhancement.SmsUpdates) || se.Equals(ServiceEnhancement.SmsAndEmailUpdates));

This might look like an overkill now but you'll be glad having this in few weeks. You never want to maintain comments. It's easier to maintain the code.

If you format it properly then it's pretty easy to read. This means that in a case like this with lenghty code I'd rather use full-property syntax, add a few line-breaks and indents and here it is. Doesn't it look much better? It documents itself, leave it as it is. Work with formatting now.

private bool NeedsMobileNumber 
{
    get
    {
        return
            ServiceEnhancements.Any(se => 
                se.Equals(ServiceEnhancement.SmsUpdates) || 
                se.Equals(ServiceEnhancement.SmsAndEmailUpdates)
            );
    }
}

---

A couple of other things...

---

public static ServiceEnhancement Loss1000 =

These should be readonly. You don't want to overwrite them by accident.

---

// This dictionary must come before the public static fields, otherwise it won't be declared in time.

Or you can use the static constructor to initialize all fields too.

---

public string GetName()
{
    return _name;
}

Why isn't this a property?

---

public static ServiceEnhancement GetByCode(string code)
{
    if (!Instance.ContainsKey(code))
    {
        throw new ArgumentException($"Code '{code}' not a valid ServiceEnhancement.", "code");
    }

    return Instance[code];
}

You can rewrite it with the new dictionary out variable which I find is easier to follow:

public static ServiceEnhancement GetByCode(string code)
{
    if (Instance.TryGetValue(code, out var serviceEnhancement))
    {
        return serviceEnhancement;
    }

    throw new ArgumentException($"Code '{code}' not a valid ServiceEnhancement.", "code");
}