2

Nhibernate forces you to use an Iesi Set, instead of the net 4 ISet interface. In the following snippet of code I check whether an iesi set contains an item:

    public virtual void Remove(Substance substance)
    {
        var test = _substances.First() == substance;

        if (!_substances.Contains(substance)) return;

        _substances.Remove(substance);
        substance.SubstanceGroup = null;
    }

The variable _substances references a HashedSet. I have added the test var just to check the code as a temporary measure. I have the Equals method overridden like this:

    public override int GetHashCode()
    {
        return Equals(Id, default(TId)) ? base.GetHashCode() : Id.GetHashCode();
    }

This causes the item to return the Id (Guid) as hash. If I check in the debugger I get the following result:

test
true
_substances.Contains(substance)
false
_substances.First().GetHashCode()
-2974953
substance.GetHashCode()
-2974953

How can it be that exactly the same object is not discovered in the collection using the contains method of that collection?? I can even do this in the debugger:

_substances.Contains(_substances.First())
false

Obviously, _substances.Remove(substance) doesn't work either. After some additional research I found out that NH replaces the collection with it's own Persistent Generic set. The problem arises when this set is used. If I retrieve an item from that set and I call Contains on the same set, it always returns false. I have overridden the GetHashCode and Equals, even put return true in the Equals method.

halcwb
  • 1,480
  • 1
  • 13
  • 26
  • are you also overriding the Equals method? like http://blog.visualt4.com/2009/03/nhibernate-why-override-gethashcode-and.html – Bas Aug 25 '11 at 13:57
  • Yes, I even put return true in it, without any result. This is another hair-pulling issue I have with Nhibernate. – halcwb Aug 25 '11 at 15:41

1 Answers1

3

There's something wrong with your Equals and GetHashCode implementation because I assure you that Iesi ISet collection works correctly. The reason it is replaced by PersistentGenericSet is that ISet is just an interface, the collection has to be replaced by a concrete type. Without more code it's hard to see where the problem is so I have pasted a better equality implementation below. One problem I can see in yours is that the hash code will change after the Id is assigned, my version handles that by caching the hash code.

public class Substance
{
    private int? _cachedHashCode;

    public Substance()
    {
        Id = Guid.Empty;
    }

    public Substance(Guid id)
    {
        Id = id;
    }

    public Guid Id { get; set; }

    public bool IsTransient
    {
        get { return Id == Guid.Empty; }
    }

    public bool Equals(Substance other)
    {
        if (IsTransient ^ other.IsTransient)
        {
            return false;
        }
        if (IsTransient && other.IsTransient)
        {
            return ReferenceEquals(this, other);
        }
        return other.Id.Equals(Id);
    }

    public override bool Equals(object obj)
    {
        if (obj == null || obj.GetType() != GetType())
        {
            return false;
        }
        var other = (Substance)obj;
        return Equals(other);
    }

    public override int GetHashCode()
    {
        if (!_cachedHashCode.HasValue)
        {
            _cachedHashCode = IsTransient ? base.GetHashCode() : Id.GetHashCode();
        }
        return _cachedHashCode.Value;
    }
}

public class Mixture
{
    public Mixture()
    {
        Substances = new HashedSet<Substance>();
    }

    public ISet<Substance> Substances { get; set; }
}

public class Tests
{
    [Test]
    public void set_contains_transient_substance()
    {
        var mixture = new Mixture();
        var s1 = new Substance();
        mixture.Substances.Add(s1);
        Assert.IsTrue(mixture.Substances.Contains(s1));
    }

    [Test]
    public void set_contains_persistent_substance()
    {
        var id = Guid.NewGuid();
        var mixture = new Mixture();

        var s1 = new Substance(id);
        mixture.Substances.Add(s1);

        var s2 = new Substance(id);
        // these were created with the same id so hash code is not cached
        // and id equality is used
        Assert.IsTrue(mixture.Substances.Contains(s2));
    }

    [Test]
    public void remove_substance()
    {
        var id = Guid.NewGuid();
        var mixture = new Mixture();

        var s1 = new Substance(id);
        mixture.Substances.Add(s1);

        var s2 = new Substance(id);
        mixture.Substances.Remove(s2);
        Assert.IsTrue(mixture.Substances.Count() == 0);
    }

    [Test]
    public void hash_code_is_cached()
    {
        var s1 = new Substance(Guid.NewGuid());
        var s2 = new Substance(Guid.NewGuid());

        var mixture = new Mixture();
        mixture.Substances.Add(s1);

        Assert.IsFalse(mixture.Substances.Contains(s2));
        // assign s1 id to s2, s2 hashcode is cached so they are not equal
        s2.Id = s1.Id;
        Assert.IsFalse(mixture.Substances.Contains(s2));
    }

}
Jamie Ide
  • 48,427
  • 16
  • 81
  • 117
  • That did the trick, thank you very much. In my base class I implemented the caching of the hashcode. – halcwb Aug 26 '11 at 08:57
  • 1
    One important caveat though, your solution implies that identifier is set by the application, not by NH. Unfortunately, you miss out on some important features of NH doing it like this. – halcwb Aug 28 '11 at 17:23