In C# 4.0, is there any way to make an otherwise p

Question

We're creating an object hierarchy where each item has a collection of other items, and each item also has a Parent property pointing to its parent item. Pretty standard stuff. We also have an ItemsCollection class that inherits from Collection<Item> which itself has an Owner property pointing to the item the collection belongs to. Again, nothing interesting there.

When an item is added to the ItemsCollection class, we want it to automatically set the parent of Item (using the collection's Owner property) and when the item is removed, we want to clear the parent.

Here's the thing. We only want the Parent setter to be available to ItemsCollection, nothing else. That way not only can we know who the parent of an item is, but we can also ensure an item isn't added to multiple collections by checking for an existing value in Parent, or letting someone arbitrarily change it to something else.

The two ways we know how to do this are:

1. Mark the setter as private, then enclose the collection definition within the scope of the item itself. Pro: Full protection. Con: Ugly code with nested classes.

2. Use a private ISetParent interface on Item that only ItemsCollection knows about. Pro: Much cleaner code and easy to follow. Con: Technically anyone who knows of the interface can cast Item and get at the setter.

Now technically via reflection anyone can get at anything anyway, but still... trying to find the best way to do this.

Now I know there was a feature in C++ called Friend or something that let you designate an otherwise private member in one class as being available to another which would be the perfect scenario, but I don't know of any such thing in C#.

In pseudocode (e.g. all the property changed notifications and such have been removed for brevity and I'm just typing this here, not copying from code), we have this...

public class Item
{
    public string Name{ get; set; }
    public Item Parent{ get; private set; }
    public ItemsCollection ChildItems;

    public Item()
    {
        this.ChildItems = new ItemsCollection (this);
    }
}

public class ItemsCollection : ObservableCollection<Item>
{
    public ItemsCollection(Item owner)
    {
        this.Owner = owner;
    }   

    public Item Owner{ get; private set; }

    private CheckParent(Item item)
    {
        if(item.Parent != null) throw new Exception("Item already belongs to another ItemsCollection");
        item.Parent = this.Owner; // <-- This is where we need to access the private Parent setter
    }

    protected override void InsertItem(int index, Item item)
    {
        CheckParent(item);
        base.InsertItem(index, item);
    }

    protected override void RemoveItem(int index)
    {
        this[index].Parent = null;
        base.RemoveItem(index);
    }

    protected override void SetItem(int index, Item item)
    {
        var existingItem = this[index];

        if(item == existingItem) return;

        CheckParent(item);
        existingItem.Parent = null;

        base.SetItem(index, item);
    }

    protected override void ClearItems()
    {
        foreach(var item in this) item.Parent = null; <-- ...as is this
        base.ClearItems();
    }

}

Any other way to do something similar?

Answer 1

You can do these sorts of things using Delegates:

public delegate void ItemParentChangerDelegate(Item item, Item newParent);

public class Item
{
    public string Name{ get; set; }
    public Item Parent{ get; private set; }
    public ItemsCollection ChildItems;

    static Item()
    {
        // I hereby empower ItemsCollection to be able to set the Parent property:
        ItemsCollection.ItemParentChanger = (item, parent) => { item.Parent = parent };
        // Now I just have to trust the ItemsCollection not to do evil things with it, such as passing it to someone else...
    }
    public static void Dummy() { }

    public Item()
    {
        this.ChildItems = new ItemsCollection (this);
    }
}

public class ItemsCollection : ObservableCollection<Item>
{
    static ItemsCollection()
    {
        /* Forces the static constructor of Item to run, so if anyone tries to set ItemParentChanger,
        it runs this static constructor, which in turn runs the static constructor of Item,
        which sets ItemParentChanger before the initial call can complete.*/
        Item.Dummy();
    }
    private static object itemParentChangerLock = new object();
    private static ItemParentChangerDelegate itemParentChanger;
    public static ItemParentChangerDelegate ItemParentChanger
    {
        private get
        {
            return itemParentChanger;
        }
        set
        {
            lock (itemParentChangerLock)
            {
                if (itemParentChanger != null)
                {
                    throw new InvalidStateException("ItemParentChanger has already been initialised!");
                }
                itemParentChanger = value;
            }
        }
    }

    public ItemsCollection(Item owner)
    {
        this.Owner = owner;
    }   

    public Item Owner{ get; private set; }

    private CheckParent(Item item)
    {
        if(item.Parent != null) throw new Exception("Item already belongs to another ItemsCollection");
        //item.Parent = this.Owner;
        ItemParentChanger(item, this.Owner); // Perfectly legal! :)
    }

    protected override void InsertItem(int index, Item item)
    {
        CheckParent(item);
        base.InsertItem(index, item);
    }

    protected override void RemoveItem(int index)
    {
        ItemParentChanger(this[index], null);
        base.RemoveItem(index);
    }

    protected override void SetItem(int index, Item item)
    {
        var existingItem = this[index];

        if(item == existingItem) return;

        CheckParent(item);
        ItemParentChanger(existingItem, null);

        base.SetItem(index, item);
    }

    protected override void ClearItems()
    {
        foreach(var item in this) ItemParentChanger(item, null);
        base.ClearItems();
    }

Answer 2

How about you make sure that only the item's current collection can orphan the item. That way no other collection can set the item's parent while it belongs to a collection. You could use a unique key of some sort so that a third party couldn't get involved:

public sealed class ItemsCollection : ObservableCollection<Item>
{
    private Dictionary<Item, Guid> guids = new Dictionary<Item, Guid>();

    public ItemsCollection(Item owner)
    {
        this.Owner = owner;
    }

    public Item Owner { get; private set; }

    private Guid CheckParent(Item item)
    {
        if (item.Parent != null)
            throw new Exception("Item already belongs to another ItemsCollection");
        //item.Parent = this.Owner; // <-- This is where we need to access the private Parent setter     
        return item.BecomeMemberOf(this);

    }

    protected override void InsertItem(int index, Item item)
    {
        Guid g = CheckParent(item);
        base.InsertItem(index, item);
        guids.Add(item, g);
    }

    protected override void RemoveItem(int index)
    {
        Item item = this[index];
        DisownItem(item);
        base.RemoveItem(index);
    }

    protected override void DisownItem(Item item)
    {
        item.BecomeOrphan(guids[item]);
        guids.Remove(item);            
    }

    protected override void SetItem(int index, Item item)
    {
        var existingItem = this[index];
        if (item == existingItem)
            return;
        Guid g = CheckParent(item);
        existingItem.BecomeOrphan(guids[existingItem]);
        base.SetItem(index, item);
        guids.Add(item, g);
    }

    protected override void ClearItems()
    {
        foreach (var item in this)
            DisownItem(item);
        base.ClearItems();
    }
}




public class Item
{
    public string Name { get; set; }
    public Item Parent { get; private set; }

    public ItemsCollection ChildItems;

    public Item()
    {
        this.ChildItems = new ItemsCollection(this);
    }

    private Guid guid;

    public Guid BecomeMemberOf(ItemsCollection collection)
    {
        if (Parent != null)
            throw new Exception("Item already belongs to another ItemsCollection");
        Parent = collection.Owner;
        guid = new Guid();
        return guid; // collection stores this privately         
    }

    public void BecomeOrphan(Guid guid) // collection passes back stored guid         
    {
        if (guid != this.guid)
            throw new InvalidOperationException("Item can only be orphaned by its current collection");
        Parent = null;
    }
}

Obviously there is redundancy there; the item collection is storing a second item collection (the dictionary). But there are numerous options for overcoming that which I assume you can think of. It's beside the point here.

However I do suggest you consider moving the task of child-item management to the item class, and keep the collection as 'dumb' as possible.

EDIT: in response to your quesion, how does this prevent and item from being in two ItemsCollections:

You ask what the point of the guids is. Why not just use the collection instance itself?

If you replace the guid argument with a collection reference, you could add an item to two different collections like this:

{
    collection1.InsertItem(item);  // item parent now == collection1
    collection2.InsertItem(item);  // fails, but I can get around it:
    item.BecomeOrphan(collection1); // item parent now == null 
    collection2.InsertItem(item);  // collection2 hijacks item by changing its parent (and exists in both collections)
}

Now imagine doing this with the guid argument:

{
    collection1.InsertItem(item);  // item parent now == collection1
    collection2.InsertItem(item);  // fails, so...
    item.BecomeOrphan(????); // can't do it because I don't know the guid, only collection1 knows it.
}

So you can't add an item to more than one ItemsCollection. And ItemsCollection is sealed so you can't subclass it and override its Insert method (and even if you did that, you still couldn't change the item's parent).

Answer 3

The only two things I can think of:

One:

Use sort of option number 2 you mention above (which I do constantly myself)...but make the implementation of the interface (Item) be a nested private class inside of ItemsCollection... that way only ItemsCollection knows about the setter. The interface IItem only declares a getter for Parent...and no one can cast it to Item because Item is private to ItemsCollection. So, something like:

public class ItemsCollection : ObservableCollection<IItem>
{
    private class Item : IItem 
    {
        public object Parent { get; set; }
    }

    private CheckParent(IItem item)
    {
        if(item.Parent != null) throw new Exception("Item already belongs to another ItemsCollection");
        ((Item)item).Parent = this.Owner; // <-- This is where we need to access the private Parent setter
    }

    public static IItem CreateItem() { return new Item(); }
}

public interface IItem 
{
    object Parent {get; }
}

and when you want ItemsCollection to set the item Parent, case the IItem instance to Item (which does expose a setter). Only ItemsCollection can do this cast, since the Item implementation is private to ItemsCollection...so I think that accomplishes what you want.

Two:

Make it internal not private...you don't get exactly what you want, but you can use InternalsVisibleToAttribute to denote that the internal members are visible to another assembly.