cakeslice / Outline-Effect

Outline Image Effect for Unity
MIT License
1.48k stars 201 forks source link

cakeslice.LinkedSet.AddOrMoveToEnd() does not work absolutely correctly #34

Closed npocmaka closed 4 years ago

npocmaka commented 6 years ago

Without knowing I was working on a third party software I've started writing unit tests for this. Here's a the test:

    [Test]
    public void LinkedSetTestWithComparer()
    {
        LinkedSet<string> ls = new LinkedSet<string>((IEqualityComparer<string>)(new MeaninglessSrtingComparer()));
        ls.Add("12");
        Assert.That(ls.Contains("34"));
        Assert.False(ls.Add("56"));
        ls.Add("1");
        var at = ls.AddOrMoveToEnd("34");
        Assert.That(ls.Contains("1"));
        var temp="";
        foreach (var s in ls) {
            temp = s;
        }
        Assert.That(temp.Equals("12"));

    }

class MeaninglessSrtingComparer : IEqualityComparer<string> {
    public bool Equals(string s1, string s2) {
        //return s1.ToString().Length == s2.ToString().Length;
        return s1.Length == s2.Length;
    }

    public int GetHashCode(string s) {
        //return s.ToString().Length;
        return s.Length;
    }

The last assertion should not fail but it is.Instead of the element "12" be moved at the end it is replaced with the new one.Which not exactly the returned result and the name of the method implie.

To me the fix looks easy:

            else if (dictionary.TryGetValue(t, out node))
            {
                list.Remove(node);
                node = list.AddLast(node.Value);
                dictionary[node.Value] = node;
                return AddType.MOVED;
            }
JimmyCushnie commented 4 years ago

This was fixed with #43 , as LinkedSet was simplified and this method was removed.