Hand off instance before constructor returns

Question

I have the habit of breaking the rules if it makes my code more concise or my API more convenient to use, and if I can thoughtfully get away with doing so in the specific circumstances. I'd like to know if I can get away with the following such infringement.

The following illustration includes a single Child and single Parent class. In fact, I am describing a common situation throughout my app. My decision regarding this problem will affect numerous classes, which is why I'm asking this question.

public sealed class Child : INotifyPropertyChanged
{
    private Child()
    {
        //initialize basic state
    }

    public Child( Parent parent )
        : this() //invoke parameterless contructor to initialize basic state
    {
        this.Parent = parent; //the last thing before the public ctor exits
    }

    public Parent Parent
    {
        get { return parent; }
        set
        {
            if ( value != parent ) {
                parent = value;
                // initialize state that depends on the presence of a Parent instance
                parent.Children.Add( this ); //hand off to parent class
                OnPropertyChanged( "Parent" );
            }
        }
    }

    // INotifyPropertyChanged impl...
}

Notice in the Parent property's setter that I hand off the instance to the Parent class. Also notice that I am invoking that setter from the public constructor.

On an earlier question I asked, it came up that handing off an instance that's not fully initialized is an anti-pattern. It may be argued that that's what's happening here. But I would say that it's not the case since basic state is initialized in the parameterless constructor which is called before the one accepting the Parent parameter.

I thought of a problem scenario arising if a Child subclass's constructor calls the base constructor with the Parent parameter. If the derived constructor initializes basic state of its own then it won't be ready in time for the hand off. But I figure I can solve that problem with the sealed keyword, if it fits the class's semantics as it does in my Child class's cases.

So am I good to go with breaking the referenced anti-pattern in these specific circumstances? Or is there some consideration that I missed?

Answer 1

You are correct, leaking this during the constructor is definitely something that you should avoid if possible.

In this case its probably fine (because you have marked the class as sealed), but why do something which looks like an anti-pattern when you can do it a better way instead?

The other nit-pick I have with your code is that the caller might not expect the act of creating a child to have added that child to the parents children, so in this case I'd probably change the pattern that you are using so that the callers code looks like this

var child = new Child();
parent.Children.Add(child);

And have the Children property set the Parent on the child when added.

Alternatively, if you really wanted to keep the above logic flow then I'd probably do something like this instead.

public sealed class Child : INotifyPropertyChanged
{
    private Child() { }

    public static Child CreateAndAddChild(Parent parent)
    {
        var child = new Child();
        child.Parent = parent;
    }

    public Parent Parent
    {
        get { return parent; }
        set
        {
            parent = value;
            parent.Children.Add( this);
            OnPropertyChanged( "Parent" );
        }
    }
}

Both of these solutions completely avoid leaking this in the constructor.

Answer 2

So am I good to go with breaking the referenced anti-pattern in these specific circumstances?

I think you're good.

I would do (have done) it myself: especially if a Child must always have a Parent, so that assigning it to the Parent is logically part of constructing the Child.

Or is there some consideration that I missed?

One difference is that if the set property throws an exception, then the object is not constructed. That makes a difference if the child is disposable.

using (Child child = new Child())
{
   Child.Parent = null; // throws an exception
   ... etc ...
} // child.Dispose is invoked

versus

using (Child child = new Child(null)) // throws an exception
{
   ... etc ...
} // child.Dispose is not invoked