protobufel / multikeymapcsharp

C# implementation of the multi-key map, allowing the search by partial key.
Apache License 2.0
6 stars 3 forks source link

Searching by partial key after removing a full key that also contained that partial key fails #6

Open adamlemmon opened 6 years ago

adamlemmon commented 6 years ago

After I remove a key via the .Remove method, calling TryGetValuesByPartialKey with a key that was contained in the full key throws an exception. There are still legitimate matching keys in the dictionary, but the method fails instead of returning them.

After removing an item by a full key, all operations should continue to work for the remaining items.

I have confirmed that the other items were returned correctly before the remove call.

I have a sample test below. Uncomment the TestMethod and Asserts and it is in valid MsTest syntax:

//[TestMethod]
public void RemoveByFullKeyTests()
{
    IMultiKeyMap<string, IEnumerable<string>, string> dict = MultiKeyMaps.CreateMultiKeyDictionary<string, IEnumerable<string>, string>();
    dict.Add(new[] { "first", "crazy", "1st" }, "one");
    dict.Add(new[] { "first", "other" }, "one again");
    dict.Add(new[] { "first", "something else" }, "one a third time");

    IEnumerable<string> values;
    //Assert.IsTrue(dict.TryGetValuesByPartialKey(new[] { "first" }, out values));
    //Assert.AreEqual(3, values.Count());

    IEnumerable<IEnumerable<string>> fullKeys = new[] { new[] { "first", "crazy", "1st" } };
    bool removedAny = false;
    foreach (var fullKey in fullKeys)
    {
        removedAny |= dict.Remove(fullKey);
    }

    //Assert.IsTrue(removedAny);

    var tryGetValuesByPartialKey = dict.TryGetValuesByPartialKey(new[] { "first" }, out values); // Fails here even though the dictionary still contains two values that should match this partial key
    //Assert.IsTrue(tryGetValuesByPartialKey);
    //Assert.AreEqual(2, values.Count());
}
adamlemmon commented 6 years ago

I've tracked it down in the code to where I believe the problem is. In the NonPositionalBaseMultiKeyMap<T, K, V> in the CreateLiteSetMultimap<TSubKey, TKey>(...) method, there is a line commented out that passes through the fullKeyComparer and instead it always passes through a reference equality comparer.

protected virtual ILiteSetMultimap<TSubKey, TKey> CreateLiteSetMultimap<TSubKey, TKey>(
            IEqualityComparer<TSubKey> subKeyComparer, IEqualityComparer<TKey> fullKeyComparer)
            where TKey : class, IEnumerable<TSubKey>
        {
            //return CreateSupportDictionary<TSubKey, ISet<TKey>>(subKeyComparer).ToSetMultimap(fullKeyComparer);//If I uncomment this line, it works and all other tests pass. Why is this line commented out?
            return CreateSupportDictionary<TSubKey, ISet<TKey>>(subKeyComparer).ToSetMultimap(EqualityComparerExtensions.ReferenceEqualityComparerOf<TKey>());
        }

If I uncomment this line, my test passes and all other tests in the project pass. Is there a reason why this line was commented out? What can I do to make this work correctly? I'm going to submit a pull request to uncomment this line, but I don't understand what the reasoning behind it being commented was. I don't want to break anyone else.