Theme by nostrich (modified).
A common (and highly recommended) pattern when mapping collections in NHibernate is to only expose a read-only version of your collections on the object, then provide granular methods for interacting with that collection, like this pseudo-code:
public class MyEntity : Entity // provides ID etc
{
public IEnumerable<MyChild> Children;
public void AddChild(MyChild child);
public void RemoveChild(MyChild child);
}
The thinking is that exposing a full IList or ICollection allows consuming classes to do things outside of the knowledge of the parent class, and it lets you do things like this, easily:
public void AddChild(MyChild child)
{
child.Parent = this;
_children.Add(child);
}
While it makes this specific scenario easier, you might be thinking that doing this for every collection on your entities is overkill. While I don't want to get too much into depth on this topic, but I liken it to using ViewModels for your front-end - a bit of effort now will save you time and pain in the long run. You do use ViewModels don't you?
Anyway, I've been seeing the benefits of doing this on a large project recently, and I've been doing it like this:
public class MyEntity : Entity // provides ID etc
{
public IEnumerable<MyChild> Children
{
get { return ChildrenCollection.AsEnumerable(); }
}
protected internal virtual ICollection<MyChild> ChildrenCollection { get; set; }
public void AddChild(MyChild child)
{
child.Parent = this;
ChildrenCollection.Add(child);
}
public void RemoveChild(MyChild child)
{
ChildrenCollection.Remove(child);
}
}
And mapping like this:
mapping.HasManyToMany(Reveal.Property<MyEntity, IEnumerable<MyChild>>("ChildrenCollection"))
.AsSet()
.Cascade.SaveUpdate();
This came from my general assumption that NHibernate wanted my collection class to be protected (and hence virtual), plus my want to make unit testing easier as by setting an InternalsVisibleTo attribute on my assemblies I can set ChildrenCollection directly and avoid going through any business logic in my Add/Remove methods.
This is a far from perfect solution (as you'll see). It's seen by some as bad practice to have internals littered about your code (though I don't see much problem in a lot of cases where unit tests are involved), plus the LINQ-to-NHibernate story was worrying. As the ChildrenCollection property is mapped and not the Children property, LING-to-NHibernate would choke on statements like the following:
session.Linq<MyEntity>().Where(e => e.Children.Contains(c));
This was because NH had no knowledge of the MyEntity.Children property. You eneded up having to open up your internals to your Data access assembly and do this:
session.Linq<MyEntity>().Where(e => e.ChildrenCollection.Contains(c));
And by opening up your domains internals to your data access assembly you are probably giving it more knowledge about your entities than you'd like.
So in an effort to tidy up this usage I did some googling and found this stackoverflow question. Applying this resulted in the following class:
public class MyEntity : Entity // provides ID etc
{
private readonly ICollection<MyChild> _children = new HashSet<MyChild>();
public IEnumerable<MyChild> Children
{
get { return _children.AsEnumerable(); }
}
public void AddChild(MyChild child)
{
child.Parent = this;
_children.Add(child);
}
public void RemoveChild(MyChild child)
{
_children.Remove(child);
}
}
And a mapping (converted from the HBM XML in the question) like this:
mapping.HasManyToMany(e => e.Children)
.Access.ReadOnlyPropertyThroughCamelCaseField(Prefix.Underscore)
.AsSet()
.Cascade.SaveUpdate();
This throws out my assumption that NH can only deal with protected virtual collections, while allowing me to work directly with the public property in my LINQ statements (eliminating one InternalsVisibleTo attribute in the process).
What about the unit testing? I decided that having the collection exposed as an internal might give me some benefit, but I should just add the methods to the entity and it didn't cause me any problems - the business logic turned out to not be a problem for tests on the classes themselves, and in other places I was using Stubs to test the objects so it actually simplified things (only having to worry about stubbing onw property instead of two).