1502

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?

2
  • 28
    @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. Commented Apr 17, 2013 at 6:50
  • 6
    You can find the article from @m.edmondson here now: codeproject.com/Articles/802375/…
    – SpeziFish
    Commented Dec 21, 2017 at 10:25

18 Answers 18

1296

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.

16
  • 187
    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]? Commented Feb 15, 2011 at 15:27
  • 127
    If you want to make sure that a derived class cannot be further derived it's perfectly acceptable to seal it.
    – Øyvind
    Commented Feb 15, 2011 at 19:40
  • 64
    @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.
    – ljs
    Commented Feb 25, 2011 at 16:37
  • 9
    @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. Commented Oct 7, 2011 at 22:29
  • 11
    @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. Commented Feb 26, 2013 at 9:16
826

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.

3
  • 64
    This is more clear than the above answer. A sample code is worth a thousand words.
    – kuradac
    Commented Jun 28, 2018 at 0:05
  • 2
    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). Commented Oct 29, 2020 at 7:16
  • 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.
    – Nyerguds
    Commented Mar 8, 2022 at 10:44
171

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!

5
  • 50
    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.
    – Lloyd
    Commented Sep 23, 2008 at 18:17
  • 4
    Actually the rules in Java are the same.
    – OlegYch
    Commented Nov 17, 2011 at 22:02
  • 10
    @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. Commented May 28, 2012 at 8:26
  • 1
    @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? Commented May 28, 2012 at 9:34
  • 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.
    – supercat
    Commented Mar 24, 2015 at 18:15
91

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();
    }
  }
1
  • 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.
    – gembird
    Commented Jan 17, 2022 at 14:08
24

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.

14

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 { }
4
  • I would be looking at using factory methods for this. Commented Mar 17, 2015 at 9:32
  • 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.
    – supercat
    Commented Mar 24, 2015 at 18:19
  • 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".
    – vgru
    Commented Mar 8, 2016 at 9:06
  • 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. Commented Oct 31, 2016 at 13:05
8

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.

6

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.

6

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.

5

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.

0
5

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);
    }
}
4
  • 4
    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 Commented Dec 23, 2015 at 15:06
  • 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 Commented Mar 29, 2018 at 10:12
  • @user1778606: absolutely! I've fixed this with your observation. Thanks! Commented Apr 2, 2018 at 19:46
  • 8
    @GustavoMori This doesn't work. The base class still calls DisplayState before the DerivedFromBetter constructor has run, so it outputs "BetterBaseClass". Commented Feb 2, 2019 at 1:34
4

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"
    }
} 
1
  • There is no 'parent' and 'child' classes, but 'base' and 'derived'.
    – hyankov
    Commented Apr 17, 2019 at 16:32
3

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

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#.

3
  • It is not forbidden to call a virtual function inside a constructor in C++.
    – qbeuek
    Commented Sep 23, 2008 at 7:32
  • The same argument holds for C++, if you don't need to access members, you don't care they weren't initialized... Commented Sep 23, 2008 at 9:17
  • 3
    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.
    – qbeuek
    Commented Sep 26, 2008 at 9:01
1

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());
    }
}
1
  • I almost never do it as it make debugging somewhat harder if you want to step in the constructor.
    – Phil1970
    Commented Apr 18, 2017 at 1:11
1

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
-2

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 :)

1
  • 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. Commented Aug 30, 2018 at 12:55
-3

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";
   }
}
3
  • You should not find a workaround, but solve the actual problem.
    – alzaimar
    Commented Jan 5, 2015 at 7:15
  • 2
    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.
    – adityap
    Commented Jan 9, 2015 at 21:00
  • @adityap To suppress warning use warning suppression jetbrains.com/help/resharper/… Commented Oct 12, 2020 at 20:50

Not the answer you're looking for? Browse other questions tagged or ask your own question.