Skip to content

Conductor<T>.Collection.OneActive/AllActive - IChild.Parent not reset on clear #465

Open
@tklingert

Description

@tklingert
void Main()
{
	var conductor = new Conductor<TestItem>.Collection.AllActive();
	
	var t1 = new TestItem();
	var t2 = new TestItem();
	var t3 = new TestItem();
	conductor.Items.Add(t1);
	conductor.Items.Add(t2);
	conductor.Items.Add(t3);
	if (t1.Parent == null || t2.Parent == null || t3.Parent == null)
		throw new Exception("t1.Parent + t2.Parent + t3.Parent should have been set by conductor");
	conductor.Items[0] = null;
	if (t1.Parent != null)
		throw new Exception("t1.Parent should have been set to <null> by conductor");
	conductor.Items.Remove(t2);
	if (t2.Parent != null) 
		throw new Exception("t2.Parent should have been set to <null> by conductor");
	conductor.Items.Clear();
	if (t3.Parent != null)
		throw new Exception("t3.Parent should have been set to <null> by conductor");
}

The code above fails on 't3.Parent != null', it seems like the conductor implementation only resets the child's parent property on a normal or index-based removal, but not on a clear operation.

The root cause seems to be located within the BindableCollection implementation:
A simple (not really pretty) workaround:

public class Workaround<T>
	where T : class
{
	private void ConductorItems_CollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
	{
		switch (e.Action)
		{
			case NotifyCollectionChangedAction.Add:
				this._mirroredItems.AddRange(e.NewItems.OfType<T>());
				break;
			case NotifyCollectionChangedAction.Remove:
				e.OldItems.OfType<T>().Apply(i => this._mirroredItems.Remove(item: i));
				break;
			case NotifyCollectionChangedAction.Replace:
				this._mirroredItems.AddRange(e.NewItems.OfType<T>());
				e.OldItems.OfType<T>().Apply(i => this._mirroredItems.Remove(item: i));
				break;
			case NotifyCollectionChangedAction.Reset:
				if (this._conductorItems.Count == 0) // clear called
				{
					this._mirroredItems.OfType<IChild>().Apply(i => i.Parent = null);
					this._mirroredItems.Clear();
					return;
				}

				// re-synchronize mirrored items
				this._mirroredItems.Where(i => !this._conductorItems.Contains(item: i)).OfType<IChild>().Apply(i => i.Parent = null);
				this._mirroredItems.Clear();
				this._mirroredItems.AddRange(collection: this._conductorItems);
				break;
		}
	}
	public Workaround(IObservableCollection<T> conductorItems)
	{
		if (conductorItems == null) throw new ArgumentNullException(nameof(conductorItems));
		this._conductorItems = conductorItems;
		this._mirroredItems = new List<T>(collection: conductorItems);
		conductorItems.CollectionChanged += this.ConductorItems_CollectionChanged;
	}
}

=>

new Workaround<TestItem>(conductor.Items);

lets the initial example pass without errors.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions