Is a private static list an appropriate way of lim

Question

I am trying to avoid multiple instances of a class being created with the same internal data. I tried an implementation with a separate class for building the MCode but trying to protect the MCode constructor did not work so I have come back to this implementation. I wonder if this is good design or what better solution there may be?

public class MCode : IEquatable<MCode>
{

    private readonly static List<MCode> Instances;
    public AEnum AE { get; }
    public byte C { get; }
    public BEnum BE { get; }

    public static MCode GetMCode(AEnum ae, BEnum be, byte c)
    {
            if (Instances==null)
            {
                Instances = new List<MCode>();
                var newmc = new MCode(ae, be, c);
                Instances.Add(newmc);
                return newmc;
            }

            var mc = Instances.Find(x => x.Equals(ae, be, c));

            if (mc == null)
            {
                var newmc = new MCode(ae, be, c);
                Instances.Add(newmc);
                return newmc;
            }
            return mc;
    }

    protected MCode(AEnum ae, BEnum be, byte c)
    {
        AE = ae;
        BE = be;
        C = c;
    }

    public new bool Equals(MCode mc)
    {
        return (GetHashCode() == mc.GetHashCode());
    }

    public new bool Equals(AEnum ae, BEnum be, byte c)
    {
        return (GetHashCode() == GetHashCode(ae, be, c));
    }

    public new int GetHashCode()
    {
        return ((byte)AE * 256 * 256 + (byte)BE * 256 * C);
    }

    public static int GetHashCode(AEnum ae, BEnum be, byte c)
    {
        return ((byte)ae * 256 * 256 + (byte)be * 256 * c);
    }
}

The motivations for this are that I have multiple instances of classes which contain the same MCode property and I'd like them all to be using the same read-only MCode instance.

Answer 1

You can achieve this with a factory method and a static dictionary:

public class MyType
{
    private readonly static Dictionary<int, MyType> instances
       = new Dictionary<int, MyType>();

    public static MyType CreateNew(int id)
    {
        if (instances.TryGetValue(id, out var instance)
            return instance;

        return new MyType(id);
    }

    private MyType(int id) { ... }

    public int UniqueId { get; }
}

If your unique identifier is something more complex than an int, I'd simply implement a private nested struct/class with value equality semantics and use that as the dictionary's key.

Answer 2

What you are describing looks to be a Flyweight Factory pattern. Flyweights are classes that are relatively small, and there are a finite number of "unique" objects, so maintaining a catalog of unique instances can help reduce unnecessarily duplicated data in memory.

One example is US State. There are only 50 unique states, so keeping a collection of 50 states as a set of unique instances might make a difference in a system that, say, needs a state for every user record.

I would also separate the class from the factory. Make the factory a separate class, and make the constructor for MCode internal (instead of protected.

I would also be careful of your Equals implementation. Just because two objects have the same hash code does not mean that they are equal. It may be true in your case since you have a finite number of objects that can be covered by the int space, but it looks weird. Implementing the actual Equals logic (which you already have in the list lookup) would also negate the need for the duplicate GetHashCode method.