Nhibernate - Initialize lists - Best Practice?

Question

Hi, 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>();
    }


}

Answer 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

Answer 2

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);
    }
}