I am trying to build a self-updating collection. Each item in the collection has a position (x,y). When the position is changed, an event is fired, and the collection will relocate the item.

Internally the collection is using a “jagged dictionary”. The outer dictionary uses the x-coordinate a key, while the nested dictionary uses the y-coordinate a key. The nested dictionary then has a list of items as value.

The collection also maintains a dictionary to store the items position as stored in the nested dictionaries – item to stored location lookup.

I am having some trouble making the collection thread safe, which I really need.

Source code for the collection:

    public class PositionCollection<TItem, TCoordinate> : ICollection<TItem>
    where TItem : IPositionable<TCoordinate>
    where TCoordinate : struct, IConvertible
{
    private readonly object itemsLock = new object();
    private readonly Dictionary<TCoordinate, Dictionary<TCoordinate, List<TItem>>> items;
    private readonly Dictionary<TItem, Vector<TCoordinate>> storedPositionLookup;

    public PositionCollection()
    {
        this.items = new Dictionary<TCoordinate, Dictionary<TCoordinate, List<TItem>>>();
        this.storedPositionLookup = new Dictionary<TItem, Vector<TCoordinate>>();
    }

    public void Add(TItem item)
    {
        if (item.Position == null)
        {
            throw new ArgumentException("Item must have a valid position.");
        }

        lock (this.itemsLock)
        {
            if (!this.items.ContainsKey(item.Position.X))
            {
                this.items.Add(item.Position.X, new Dictionary<TCoordinate, List<TItem>>());
            }

            Dictionary<TCoordinate, List<TItem>> xRow = this.items[item.Position.X];

            if (!xRow.ContainsKey(item.Position.Y))
            {
                xRow.Add(item.Position.Y, new List<TItem>());
            }

            xRow[item.Position.Y].Add(item);

            if (this.storedPositionLookup.ContainsKey(item))
            {
                this.storedPositionLookup[item] = new Vector<TCoordinate>(item.Position);
            }
            else
            {
                this.storedPositionLookup.Add(item, new Vector<TCoordinate>(item.Position)); // Store a copy of the original position
            }

            item.Position.PropertyChanged += (object sender, PropertyChangedEventArgs eventArgs) => this.UpdatePosition(item, eventArgs.PropertyName);
        }
    }

    private void UpdatePosition(TItem item, string propertyName)
    {
        lock (this.itemsLock)
        {
            Vector<TCoordinate> storedPosition = this.storedPositionLookup[item];
            this.RemoveAt(storedPosition, item);
            this.storedPositionLookup.Remove(item);
        }
    }
}

I have written a simple unit test to check for concurrency issues:

        [TestMethod]
    public void TestThreadedPositionChange()
    {
        PositionCollection<Crate, int> collection = new PositionCollection<Crate, int>();
        Crate crate = new Crate(new Vector<int>(5, 5));
        collection.Add(crate);

        Parallel.For(0, 100, new Action<int>((i) => crate.Position.X += 1));

        Crate same = collection[105, 5].First();
        Assert.AreEqual(crate, same);
    }

The actual stored position varies every time I run the test. I appreciate any feedback you may have.