Nhibernate - Initialize lists - Best Practice?

Go To StackoverFlow.com

6

I was just wondering about some CodeWarning (ConstructorsShouldNotCallBaseClassVirtualMethods), and if there is a better way to do it. I have a simple log collector class, and I am using NHibernate to retrieve some of the objects.

Some times I create objects by myself (of course) and add them to NHibernate for persistance. What is the best way to make sure that Lists are never NULL.

Currently I am doing this, but it does not seems "perfect". Any idea on this topic?

public class LogRun
{
    public virtual int Id { get; private set; }
    public virtual DateTime StartTime { get; set; }
    public virtual DateTime EndTime { get; set; }
    public virtual IList<Log> LogMessages { get; set; }
    public virtual int LogMessageCount { get { return LogMessages.Count; } }

    public LogRun()
    {
        LogMessages = new List<Log>();
    }


}
2009-06-16 13:44
by Christian Ruppert


8

Is LogMessages a persisted thing? If so, it's best practice to never expose a public setter. NHibernate gets weird if you retreive from the database and then replace that IList with a new one:

var myLog = session.Get<LogRun>(1);
Assert.True(myLog.LogMessages.Count > 0);
myLog.LogMessages = new List<Log>();

If you note, NHibernate is returning a proxied object and replacing it with a generic list will cause it to go wonky when you try and save back.

As a rule, I prefer to have a private field that I initialize and then expose only a getter to the client:

public class LogRun
{
    private IList<Log> logMessages = new List<Log>();

    public virtual int Id { get; private set; } 
    public virtual DateTime StartTime { get; set; } 
    public virtual DateTime EndTime { get; set; }
    public virtual IList<Log> LogMessages { get { return logMessages; } } 
    public virtual int LogMessageCount { get { return LogMessages.Count; } }

    public void AddLogMessage(Log log)
    {
        logMessages.Add(log);
    }
}

Actually, I go a step further, the client gets an IEnumerable<> and I add a helper function for the add.

My implmentation would look like

public class LogRun
{
    private IList<Log> logMessages = new List<Log>();

    public virtual int Id { get; private set; } 
    public virtual DateTime StartTime { get; set; } 
    public virtual DateTime EndTime { get; set; }
    public virtual IEnumerable<Log> LogMessages { get { return logMessages; } } 
    public virtual int LogMessageCount { get { return LogMessages.Count(); } }

    public void AddLogMessage(Log log)
    {
        logMessages.Add(log);
    }
}
2009-06-16 14:48
by Ben
I follow the same pattern except I do all my initialization in the constructor. In addition, it's typically necessary to set the reference to the parent in the add method, i.e. logMessages.Add(log); log.LogRun = this - Jamie Ide 2009-06-16 14:52
I go a step further and make my getter return a read only version... return logMessages.ToList().AsReadOnly() - Webjedi 2009-06-16 14:53
Jamie Ide: True, if it's a one-many with a parent then that helper is a great place to set it. If it's a Many to many then there isn't a parent. Not sure on context here but highly suspect you're right.

Webjedi, I like your approach too. That's why I ended up at Enumerable. In my case, it satisfied everything I wanted to do with the collection - Ben 2009-06-16 14:58

good answer, except your helper function needs to be virtual for it to work - Xcalibur 2012-03-15 09:04
Keep in mind to alter access type to field in your mapping (either .Access.Field() in fluent or access="field" for XML-mappings - falstaff 2013-01-25 15:06


1

I do the same thing, but I also wonder how big the perfomance impact is, since NHibernate will also create a new List<> for every default constructor call..

I think we're in luck though, and that it will work. Consider NHibernate creating a lazy-loaded list of LogRun's (which is why we mark everything as virtual anyway):

  1. NHibernate will reflect over LogRun and create a derived class
  2. NHibernate will make a proxy-list of the LogRun-derived class
  3. --
  4. When you load that proxy, it will instantiate some of those derived classes, however, the base-constructor is called first - creating the new list<> - then the derived constructor is called, creating a proxy-list instead.

Effectively, we have created a list that we will never use.

Consider the alternatives though:

  • Make the constructor protected, so that no one will call it, and make an alternative. For example, a static LogRun.GetNew(); method.
  • Allow public set-access to the IList<> and create it yourself whenever you create a new object.

Honestly, I think both are very messy, and since I'm (pretty) sure that the perfomance overhead on creating a new empty list on each constructor-call is limited, that's what I'm personally sticking too so far.. Well, at least untill my profiler tells me otherwise :P

2009-06-16 13:56
by cwap
Ads