Grow-only set (CRDT)

Question

I made up a quick and dirty implementation of the G-Set in C#. I was hoping for some feedback on my approach. I'm not totally happy with having the payload public but I can't see that I'm gaining much by making it private either.

public class GrowOnlySet<T>
{
    public HashSet<T> payload {get; private set;}

    public GrowOnlySet()
    {
        payload = new HashSet<T>();
    }

    public GrowOnlySet(HashSet<T> newstore)
    {
        payload = newstore;
    }

    public void Add(T element)
    {
        payload.Add(element);
    }

    public bool Lookup(T element)
    {
        return payload.Contains(element);
    }

    public bool Compare(GrowOnlySet<T> R)
    {
        return R.payload.IsSubsetOf(this.payload);
    }

    public void Merge(GrowOnlySet<T> R)
    {
        this.payload.UnionWith(R.payload);
    }
}

Answer 1

I can't see that I'm gaining much by making it private either.

The clients of the GrowOnlySet might use this object in a inappropriate way. For example they can remove an element.

So I can do this, the compiler lets me do it.

var gset = new GrowOnlySet<string>();
gset.payload.Remove("element");

Now there is no grow only set anymore. The encapsulation is important.

Also as a client I really don't need to know the internal representation, which is the way how it was chose to represent the grow only set.

And because the payload property is public now the GrowOnlySet interface has a lot of other methods that I can use (via .payload... ), thus there is no abstraction. But I needed an interface made of only Lookup, Compare, Merge and Add methods.

There is not check for null newstore.

 public GrowOnlySet(HashSet<T> newstore)
    {
        payload = newstore;
    }

Thus the methods that are using the payload will throw a NullReferenceException which is not good. I prefer to throw a ArgumentNullException so I can create a consistent GrowOnlySet and to let the clients to be aware that they need to provide a new store.

 public bool Compare(GrowOnlySet<T> R)
    {
        return R.payload.IsSubsetOf(this.payload);
    }

Naming convention here. Suggested name is r. Or maybe find a better name like other (inspired from the HashSet itself)

this. is redundant, so it can be removed to make the code cleaner.

Compare and Merge accept null arguments but again this will throw NullReferenceException. Better is to inform the clients of this class that it really needs an object to be compared or to be merged with.

So the class is refactored to something like this

 public class GrowOnlySet<T>
    {
        private readonly HashSet<T> _payload;

        public GrowOnlySet()
        {
            _payload = new HashSet<T>();
        }

        public GrowOnlySet(HashSet<T> newstore)
        {            
            _payload = newstore ?? throw new ArgumentNullException(nameof(newstore));
        }

        public void Add(T element)
        {
            _payload.Add(element);
        }

        public bool Lookup(T element)
        {
            return _payload.Contains(element);
        }

        public bool Compare(GrowOnlySet<T> other)
        {
            if (other == null) throw new ArgumentNullException(nameof(other));

            return other._payload.IsSubsetOf(_payload);
        }

        public void Merge(GrowOnlySet<T> other)
        {
            if (other == null) throw new ArgumentNullException(nameof(other));

            _payload.UnionWith(other._payload);
        }
    }