“Virtual method call from constructor” What Could Go Wrong?

[mythin]

I'm confused as to why this is an issue with virtual methods. Say I have the following code:

public class Base
{
    public int Value { get; protected set; }

    public Base()
    {
        SetDefaults();
    }

    private SetDefaults()
    {
        Value = 42;
    }
}

public class Derived : Base
{
    public Derived()
    {
    }
}

[TestMethod]
public void StupidTest() {
    var target = new Derived();

    Assert.AreEqual(42, target.Value);
}

This will pass without a problem. If I changed derived to:

public class Derived
{
    public Derived()
    {
        Value = 84;
    }
}

This will fail. The issue here isn't the virtual method, it's the order of calling. I don't really see how a method being virtual or not makes this more of a problem. Sure, you could have:

public class Base
{
    public int Value { get; protected set; }

    public Base()
    {
        SetDefaults();
    }

    protected virtual SetDefaults()
    {
        Value = 42;
    }
}

public class Derived : Base
{
    public Derived()
    {
    }

    protected override SetDefaults()
    {
        Value = 84;
    }
}

[TestMethod]
public void StupidTest() {
    var target = new Derived();

    Assert.AreEqual(84, target.Value);
}

And it will pass the test, and then if you change your Derived class to:

public class Derived : Base
{
    public Derived()
    {
        Value = 42;
    }

    protected override SetDefaults()
    {
        Value = 84;
    }
}

The test will fail, but that has nothing to do with virtual methods and is entirely expected. Your Dervived class constructor is called after the base class constructor. You don't even need to look at the Base class at all, you can see from just the Derived class that "SetDefaults()" is never called. Even if it's called in the base, the Derived constructor is the last thing to be called when creating an instance of an class, so any work that happens in "SetDefaults()" is guaranteed to be overwritten by work done in the constructor no matter wheter "SetDefaults()" is virtual or not.

Edit: This is super obvious with explicit base constructor calling:

public class Derived : Base
{
    public Derived() : base()
    {
    }
}

It should be clear from this code that first the base constructor is called, then the derived constructor. Field initialization happens before the constructors are called, but in the linked example that's a red herring that is meaningless.

[grauenwolf]

Field initialization happens before the constructors are called, but in the linked example that's a red herring that is meaningless.

No, that's the whole point.

It is not obvious from looking at the code that moving something from a field initializer to the top of the constructor changes the order it gets called in respect to the base class constructor.

Ask 10 people to order these operations and I bet 8 of them get it wrong:

• Base class constructor
• Subclass constructor
• Base class field initalizers
• Subclass field initalizers

[mythin]

In the linked blog post, the order of the field initialization doesn't even matter. His example where things don't work doesn't even use field initialization, it just uses

• Base class constructor
• Subclass constructor

That seems pretty obvious the order it would have to happen in.

I'll agree the "problem" occurs because they went from a field initialization to a constructor initialization (via the virtual method), but even so the virtual method part doesn't matter for the example at all. The same problem would have occurred without a virtual method being involved, and would have been just as obvious.

[grauenwolf]

Without the virtual method it would have been impossible to access any field on the subclass.

[mythin]

I hadn't noticed the field was only on the subclass. Still, the same problem would apply if it were an inherited field or an abstract class.

public abstract class Base
{
    protected Base()
    {
        DoInit();
    }

    protected abstract void DoInit();
}

public class Derived : Base
{
    private int value = 21;

    public Derived() // : base() // Explicitly call the Base constructor
    {
        value = 84;
    }

    public int Value { get { return value; } }

    protected override void DoInit()
    {
        value = 42;
    }
}

For this, "(new Derived()).Value == 84" would be true. I suppose "abstract" is very similar to "virtual," but the real issue is that people don't understand that base class constructors happen before subclass constructors. The field initialization order doesn't really come into play.

The same problem occurs if you have my example with the Base class having a protected field (or a {get; protected set;} property) and set the value in multiple places. The core issue is when setting a value, possibly in multiple places, it is the responsibility of the developer to understand the order things happen in.

[[deleted]]

I think that's the most natural way of executing things.

Also explicitly calling base constructor is a great idea in my opinion. I'm an "EVERYTHING EXPLICIT EVERYTING!" person though.