Virtual member call in a constructor

Question

I'm getting a warning from ReSharper about a call to a virtual member from my objects constructor.

Why would this be something not to do?

Henig

@m.edmondson, Seriously.. your comment should be the answer here. While Greg explanation is correct I did not understand it until I read your blog. – Tasia

You can find the article from @m.edmondson here now: codeproject.com/Articles/802375/… – Riendeau

Answer 1

When an object written in C# is constructed, what happens is that the initializers run in order from the most derived class to the base class, and then constructors run in order from the base class to the most derived class (see Eric Lippert's blog for details as to why this is).

Also in .NET objects do not change type as they are constructed, but start out as the most derived type, with the method table being for the most derived type. This means that virtual method calls always run on the most derived type.

When you combine these two facts you are left with the problem that if you make a virtual method call in a constructor, and it is not the most derived type in its inheritance hierarchy, that it will be called on a class whose constructor has not been run, and therefore may not be in a suitable state to have that method called.

This problem is, of course, mitigated if you mark your class as sealed to ensure that it is the most derived type in the inheritance hierarchy - in which case it is perfectly safe to call the virtual method.

Eachern

Greg, please tell me why would anyone have a class SEALED (which cannot be INHERITED) when it has VIRTUAL members [that is to override in DERIVED classes]? – Dudden

If you want to make sure that a derived class cannot be further derived it's perfectly acceptable to seal it. – Muscovite

@Paul - The point is that have finished deriving the virtual members of the base class[es], and thus are marking the class as fully derived as you want it to be. – Contusion

@Greg If the virtual method's behaviour has nothing to do with the instance variables, isn't this ok? It seems like maybe we should be able to declare that a virtual method will not modify the instance variables? (static?) For example, if you want to have a virtual method that can be overridden to instantiate a more derived type. This seems safe to me, and does not warrant this warning. – Gravante

@Sahuagin - It is possible to write a virtual method that can be called from a constructor without any problems, and you can add the //ReSharper Warning Disable comment to stop it complaining. But R# is just warning you that it's potentially dangerous. As always, warnings are sometimes safe to ignore if you know what you're doing. – Eachern

Just curious. If you didn't want to seal the class, what would be the appropriate way to get/set a virtual member property in a constructor? Would you need a wrapper method/property with logic to account for the possibility of the member to have been overridden? – Foretopgallant

@Foretopgallant - Calling anything which is not guaranteed to be at the top of the inheritance chain either directly or indirectly could cause problems. If you didn't want to seal the entire class but wanted to call a particular virtual method or use a particular virtual property from the constructor you could override and seal the members you require access to, e.g. protected sealed override void Foo() { ... }. – Eachern

@PaulPacurar - If you want to call a virtual method in the most derived class, you still get the warning while you know it's not going to cause a problem. In that case, you could share your knowledge with the system by sealing that class. – Hardhearted

There is one case that you might call virtual members in constructor. When you use some ORM frameworks that require the properties of entity classes to be virtual. And you might initialize property which has one to many relation to the entity in constructor to give it a default value. That's where I got the warning and find this post. I think we have nothing to do with that. – Juliettejulina

I rolled back the edit that was made because it was incorrect. Quite how it got approved I'll never know. – Eachern

If you don't want to seal the class and you want to use the method from the inherited base class then call base.Method(); – Abydos

@CADbloke, no that's not how it works when the method is virtual. – Eachern

Ah, ok thanks. I'll leave this here as a warning to others (regarding virtual methods as well as my comments). – Abydos

I rolled this back because the edit was wrong. When I said "initializers" I quite explicitly meant that term. Clarifying that it has "nothing to do with object I initializers" when that's exactly what I'm talking about is not a clarification, it's just making it incorrect. – Eachern

A SOLUTION - If you're calling a virtual method in a constructor, you're probably aware by now that you are manipulating the object life cycle. And I'd wager the class you're creating could be described as a "helper proxy". - You want to make sure that the inherited class has an opportunity to initialize itself. - So create a virtual void Initialize() method and let that be the first method called in the base class constructor. - Then the subclass can choose to override the Initialize method for any constructor-like operations. - You see this pattern in MVC controllers in .Net – Misrepresent

so the explanation is very clear and straightforward, but the solution is...? simply you have to avoid doing this? and if you need to do this, the explanation is you have poorly designed? – Indignant

Answer 2

In order to answer your question, consider this question: what will the below code print out when the Child object is instantiated?

class Parent
{
    public Parent()
    {
        DoSomething();
    }

    protected virtual void DoSomething() 
    {
    }
}

class Child : Parent
{
    private string foo;

    public Child() 
    { 
        foo = "HELLO"; 
    }

    protected override void DoSomething()
    {
        Console.WriteLine(foo.ToLower()); //NullReferenceException!?!
    }
}

The answer is that in fact a NullReferenceException will be thrown, because foo is null. An object's base constructor is called before its own constructor. By having a virtual call in an object's constructor you are introducing the possibility that inheriting objects will execute code before they have been fully initialized.

Fumikofumitory

This is more clear than the above answer. A sample code is worth a thousand words. – Normalie

I think initialize foo inplace (like private string foo="INI";) would make it more clear that foo does get initialized. (instead of some uninitialized state). – Collation

Excellent example to show the danger. But, to demonstrate a safe variant of this situation, if DoSomething() just executes Console.WriteLine("hello"); without accessing any local variables, there is no issue. – Northnortheast

Answer 3

The rules of C# are very different from that of Java and C++.

When you are in the constructor for some object in C#, that object exists in a fully initialized (just not "constructed") form, as its fully derived type.

namespace Demo
{
    class A 
    {
      public A()
      {
        System.Console.WriteLine("This is a {0},", this.GetType());
      }
    }

    class B : A
    {      
    }

    // . . .

    B b = new B(); // Output: "This is a Demo.B"
}

This means that if you call a virtual function from the constructor of A, it will resolve to any override in B, if one is provided.

Even if you intentionally set up A and B like this, fully understanding the behavior of the system, you could be in for a shock later. Say you called virtual functions in B's constructor, "knowing" they would be handled by B or A as appropriate. Then time passes, and someone else decides they need to define C, and override some of the virtual functions there. All of a sudden B's constructor ends up calling code in C, which could lead to quite surprising behavior.

It is probably a good idea to avoid virtual functions in constructors anyway, since the rules are so different between C#, C++, and Java. Your programmers may not know what to expect!

Mikaelamikal

Greg Beech's answer, while unfortunately not voted as high as my answer, I feel is the better answer. It's certainly got a few more valuable, explanatory details I didn't take the time to include. – Mikaelamikal

Actually the rules in Java are the same. – Sputum

@JoãoPortela C++ is very different actually. Virtual method calls in constructors (and destructors!) are resolved using the type (and vtable) currently being constructed, not the most derived type as Java and C# both do. Here is the relevant FAQ entry. – Musician

@JacekSieka you are absolutely correct. It's been a while since I coded in C++ and I somehow confused all this. Should I delete the comment to avoid confusing anyone else? – Pentobarbital

There is a significant way in which C# is different from both Java and VB.NET; in C#, fields which are initialized at the point of declaration will have their initializations processed before the base constructor call; this was done for the purpose of allowing derived-class objects to be usable from the constructor, but unfortunately such ability only works for derived-class features whose initialization isn't controlled by any derived-class parameters. – Trigonometry

Answer 4

Reasons of the warning are already described, but how would you fix the warning? You have to seal either class or virtual member.

  class B
  {
    protected virtual void Foo() { }
  }

  class A : B
  {
    public A()
    {
      Foo(); // warning here
    }
  }

You can seal class A:

  sealed class A : B
  {
    public A()
    {
      Foo(); // no warning
    }
  }

Or you can seal method Foo:

  class A : B
  {
    public A()
    {
      Foo(); // no warning
    }

    protected sealed override void Foo()
    {
      base.Foo();
    }
  }

Brinkley

Or could just explicitly say in constructor of class A : A() { base.Foo(); } then the Foo() of base class B will be always called in constructor of A. – Excitor

Answer 5

In C#, a base class' constructor runs before the derived class' constructor, so any instance fields that a derived class might use in the possibly-overridden virtual member are not initialized yet.

Do note that this is just a warning to make you pay attention and make sure it's all-right. There are actual use-cases for this scenario, you just have to document the behavior of the virtual member that it can not use any instance fields declared in a derived class below where the constructor calling it is.

Zurn

Answer 6

There are well-written answers above for why you wouldn't want to do that. Here's a counter-example where perhaps you would want to do that (translated into C# from Practical Object-Oriented Design in Ruby by Sandi Metz, p. 126).

Note that GetDependency() isn't touching any instance variables. It would be static if static methods could be virtual.

(To be fair, there are probably smarter ways of doing this via dependency injection containers or object initializers...)

public class MyClass
{
    private IDependency _myDependency;

    public MyClass(IDependency someValue = null)
    {
        _myDependency = someValue ?? GetDependency();
    }

    // If this were static, it could not be overridden
    // as static methods cannot be virtual in C#.
    protected virtual IDependency GetDependency() 
    {
        return new SomeDependency();
    }
}

public class MySubClass : MyClass
{
    protected override IDependency GetDependency()
    {
        return new SomeOtherDependency();
    }
}

public interface IDependency  { }
public class SomeDependency : IDependency { }
public class SomeOtherDependency : IDependency { }

Levee

I would be looking at using factory methods for this. – Apparatus

I wish the .NET Framework had, instead of including the mostly-useless Finalize as a default member of Object, had used that vtable slot for a ManageLifetime(LifetimeStatus) method which would get called when a constructor returns to client code, when a constructor throws, or when an object is found to be abandoned. Most scenarios which would entail calling a virtual method from a base-class constructor could be best handled using two-stage construction, but two-stage construction should behave as an implementation detail, rather than a requirement that clients invoke the second stage. – Trigonometry

Still, issues can arise with this code just as with any other case shown in this thread; GetDependency is not guaranteed to be safe for invoking before MySubClass constructor has been called. Also, having default dependencies instantiated by default is not what you'd call "pure DI". – Spillage

The example does "dependency outjection". ;-) To me this is yet another good counter-example for a virtual method invocation from a constructor. SomeDependency is no longer instantiated in MySubClass derivations leading to broken behavior for every MyClass feature that depends on SomeDependency. – Semipalmate

Answer 7

Yes, it's generally bad to call virtual method in the constructor.

At this point, the objet may not be fully constructed yet, and the invariants expected by methods may not hold yet.

Touber

Answer 8

Because until the constructor has completed executing, the object is not fully instantiated. Any members referenced by the virtual function may not be initialised. In C++, when you are in a constructor, this only refers to the static type of the constructor you are in, and not the actual dynamic type of the object that is being created. This means that the virtual function call might not even go where you expect it to.

Buttons

Answer 9

Your constructor may (later, in an extension of your software) be called from the constructor of a subclass that overrides the virtual method. Now not the subclass's implementation of the function, but the implementation of the base class will be called. So it doesn't really make sense to call a virtual function here.

However, if your design satisfies the Liskov Substitution principle, no harm will be done. Probably that's why it's tolerated - a warning, not an error.

Solvency

Answer 10

One important aspect of this question which other answers have not yet addressed is that it is safe for a base-class to call virtual members from within its constructor if that is what the derived classes are expecting it to do. In such cases, the designer of the derived class is responsible for ensuring that any methods which are run before construction is complete will behave as sensibly as they can under the circumstances. For example, in C++/CLI, constructors are wrapped in code which will call Dispose on the partially-constructed object if construction fails. Calling Dispose in such cases is often necessary to prevent resource leaks, but Dispose methods must be prepared for the possibility that the object upon which they are run may not have been fully constructed.

Trigonometry

Answer 11

One important missing bit is, what is the correct way to resolve this issue?

As Greg explained, the root problem here is that a base class constructor would invoke the virtual member before the derived class has been constructed.

The following code, taken from MSDN's constructor design guidelines, demonstrates this issue.

public class BadBaseClass
{
    protected string state;

    public BadBaseClass()
    {
        this.state = "BadBaseClass";
        this.DisplayState();
    }

    public virtual void DisplayState()
    {
    }
}

public class DerivedFromBad : BadBaseClass
{
    public DerivedFromBad()
    {
        this.state = "DerivedFromBad";
    }

    public override void DisplayState()
    {   
        Console.WriteLine(this.state);
    }
}

When a new instance of DerivedFromBad is created, the base class constructor calls to DisplayState and shows BadBaseClass because the field has not yet been update by the derived constructor.

public class Tester
{
    public static void Main()
    {
        var bad = new DerivedFromBad();
    }
}

An improved implementation removes the virtual method from the base class constructor, and uses an Initialize method. Creating a new instance of DerivedFromBetter displays the expected "DerivedFromBetter"

public class BetterBaseClass
{
    protected string state;

    public BetterBaseClass()
    {
        this.state = "BetterBaseClass";
        this.Initialize();
    }

    public void Initialize()
    {
        this.DisplayState();
    }

    public virtual void DisplayState()
    {
    }
}

public class DerivedFromBetter : BetterBaseClass
{
    public DerivedFromBetter()
    {
        this.state = "DerivedFromBetter";
    }

    public override void DisplayState()
    {
        Console.WriteLine(this.state);
    }
}

Hennessy

um, I think the DerivedFromBetter constructor calls the BetterBaseClass constructor implicity. the code above should be equivalent to public DerivedFromBetter() : base(), so intialize would be called twice – Sickroom

You could define a protected constructor in the BetterBaseClass class that has an additional bool initialize parameter, which determines whether Initializeis called in the base constructor. The derived constructor would then call base(false) to avoid calling Initialize twice – Outlet

@user1778606: absolutely! I've fixed this with your observation. Thanks! – Hennessy

@GustavoMori This doesn't work. The base class still calls DisplayState before the DerivedFromBetter constructor has run, so it outputs "BetterBaseClass". – Kitchenette

Answer 12

The warning is a reminder that virtual members are likely to be overridden on derived class. In that case whatever the parent class did to a virtual member will be undone or changed by overriding child class. Look at the small example blow for clarity

The parent class below attempts to set value to a virtual member on its constructor. And this will trigger Re-sharper warning, let see on code:

public class Parent
{
    public virtual object Obj{get;set;}
    public Parent()
    {
        // Re-sharper warning: this is open to change from 
        // inheriting class overriding virtual member
        this.Obj = new Object();
    }
}

The child class here overrides the parent property. If this property was not marked virtual the compiler would warn that the property hides property on the parent class and suggest that you add 'new' keyword if it is intentional.

public class Child: Parent
{
    public Child():base()
    {
        this.Obj = "Something";
    }
    public override object Obj{get;set;}
}

Finally the impact on use, the output of the example below abandons the initial value set by parent class constructor. And this is what Re-sharper attempts to to warn you, values set on the Parent class constructor are open to be overwritten by the child class constructor which is called right after the parent class constructor.

public class Program
{
    public static void Main()
    {
        var child = new Child();
        // anything that is done on parent virtual member is destroyed
        Console.WriteLine(child.Obj);
        // Output: "Something"
    }
} 

Shellashellac

There is no 'parent' and 'child' classes, but 'base' and 'derived'. – Stoneware

Answer 13

Beware of blindly following Resharper's advice and making the class sealed! If it's a model in EF Code First it will remove the virtual keyword and that would disable lazy loading of it's relationships.

    public **virtual** User User{ get; set; }

Dorinedorion

Answer 14

There's a difference between C++ and C# in this specific case. In C++ the object is not initialized and therefore it is unsafe to call a virutal function inside a constructor. In C# when a class object is created all its members are zero initialized. It is possible to call a virtual function in the constructor but if you'll might access members that are still zero. If you don't need to access members it is quite safe to call a virtual function in C#.

Dodd

It is not forbidden to call a virtual function inside a constructor in C++. – Misvalue

The same argument holds for C++, if you don't need to access members, you don't care they weren't initialized... – Touber

No. When you call a virtual method in a constructor in C++ it will invoke not the deepest overriden implementation, but the version associated with the current type. It is called virtually, but as if on a type of the current class - you don't have access to methods and members of a derived class. – Misvalue

Answer 15

Just to add my thoughts. If you always initialize the private field when define it, this problem should be avoid. At least below code works like a charm:

class Parent
{
    public Parent()
    {
        DoSomething();
    }
    protected virtual void DoSomething()
    {
    }
}

class Child : Parent
{
    private string foo = "HELLO";
    public Child() { /*Originally foo initialized here. Removed.*/ }
    protected override void DoSomething()
    {
        Console.WriteLine(foo.ToLower());
    }
}

Gujral

I almost never do it as it make debugging somewhat harder if you want to step in the constructor. – Letters

Answer 16

I think that ignoring the warning might be legitimate if you want to give the child class the ability to set or override a property that the parent constructor will use right away:

internal class Parent
{
    public Parent()
    {
        Console.WriteLine("Parent ctor");
        Console.WriteLine(Something);
    }

    protected virtual string Something { get; } = "Parent";
}

internal class Child : Parent
{
    public Child()
    {
        Console.WriteLine("Child ctor");
        Console.WriteLine(Something);
    }

    protected override string Something { get; } = "Child";
}

The risk here would be for the child class to set the property from its constructor in which case the change in the value would occur after the base class constructor has been called.

My use case is that I want the child class to provide a specific value or a utility class such as a converter and I don't want to have to call an initialization method on the base.

The output of the above when instantiating the child class is:

Parent ctor
Child
Child ctor
Child

Bellina

Answer 17

I would just add an Initialize() method to the base class and then call that from derived constructors. That method will call any virtual/abstract methods/properties AFTER all of the constructors have been executed :)

Jackjackadandy

That makes the warning disappear but does not fix the problem. You run into the same problem as others explained, when you add a more derived class. – Unexperienced

Answer 18

Another interesting thing I found is that the ReSharper error can be 'satisfied' by doing something like below which is dumb to me. However, as mentioned by many earlier, it still is not a good idea to call virtual properties/methods in constructor.

public class ConfigManager
{
   public virtual int MyPropOne { get; private set; }
   public virtual string MyPropTwo { get; private set; }

   public ConfigManager()
   {
    Setup();
   }

   private void Setup()
   {
    MyPropOne = 1;
    MyPropTwo = "test";
   }
}

Sacral

You should not find a workaround, but solve the actual problem. – Autoroute

I agree @alzaimar! I am trying to leave options for the persons facing similar issue and who does not want to implement the solutions provided above, probably due to some limitations. With this (as I mentioned in my workaround above), another thing that I am trying to point out is that ReSharper, if possible, needs to be able to flag this workaround as error too. However it currently does not which could lead to two things - they forgot about this scenario or they wanted to deliberately leave it out out for some valid use case one cannot think of right now. – Sacral

@Sacral To suppress warning use warning suppression jetbrains.com/help/resharper/… – Trishatriskelion