Is it OK to call virtual properties from the constructor of a NHibernate entity?

Question

take a look at this example code:

public class Comment
{
    private Comment()
    { }

    public Comment(string text, DateTime creationDate, string authorEmail)
    {
        Text = text;
        CreationDate = creationDate;
        AuthorEmail = authorEmail;
    }

    public virtual string Text { get; private set; }
    public virtual DateTime CreationDate { get; set; }
    public virtual string AuthorEmail { get; private set; }
}

i know it's considered bad practice to call virtual member functions from the constructor, however in NHibernate i need the properties to be virtual to support lazy loading. Is it considered OK in this case?

Answer 1

I'm pretty sure this is fine, but if your worried you could always just assign the properties after a parameter less constructor call.

Answer 2

To expand on Paco's answer:

In most cases it doesn't hurt. But if the class is inherited, virtual allows the properties get/set to be overriden, so the behavior is no longer fully encapsulated and controlled, so it can break, in theory. FxCop warns about this because it's a potential problem.

The point of FxCop is to help warn you about potential problems though. It is not wrong to use properties in a constructor if you know you who/what is ever going to inherit from the class, but it isn't officially 'best practice'.

So, the answer is that it's fine as long as you control any inheritence of the class. Otherwise, don't use it and set the field values directly. (Which means you can't use C# 3.0 automatic get/set properties--you'll have to write properties wrapping fields yourself.)

Side note: Personally, all of my projects are web sites that we host for clients. So assuming this setup stays the same for a project, than it's worth the trade-off of having to duplicate the various null/argument checking. But, in any other case where I am not sure that we'll maintain complete control of the project and use of the class, I wouldn't take this shortcut.

Answer 3

It's OK in this sample, but it might cause problems when you inherit the class and override the properties. Generally, you can better create fields for the virtual properties.

Answer 4

IMHO the best-practice is to use properties with backing fields:

public class Comment
{
    private DateTime _creationDate;
    private string _text;
    private string _authorEmail;
    private Comment() { }
    public Comment(string text, DateTime creationDate, string authorEmail)
    {
        _text = text;
        _creationDate = creationDate;
        _authorEmail = authorEmail;
    }
    public virtual string Text
    {
        get { return _text; }
        private set { _text = value; }
    }
    public virtual string AuthorEmail
    {
        get { return _authorEmail; }
        private set { _authorEmail = value; }
    }
    public virtual DateTime CreationDate
    {
        get { return _creationDate; }
        set { _creationDate = value; }
    }
}

So you can avoid problems on child classes and you don't see any warning anymore