What's Wrong With This Code? (#4)

Tuesday, August 29, 2006

This program throws an exception at runtime. If you've been burned by this problem before, you'll know what's wrong before you even see the definition for class Bar...

using System;

class Program
{
  
static void Main()
  {
    
Foo foo = new Bar();
  }
}

abstract class Foo
{
  
public Foo()
  {
    Init();
  }

  
protected abstract void Init();
}

class Bar : Foo
{
  
string message;

  
public Bar()
  {
    message =
"Hello!";
  }

  
protected override void Init()
  {
    
Console.WriteLine(message.ToUpper());
  }
}


Comments
Grim Tuesday, August 29, 2006
NullReferenceException

Because the base class constructor executes before the child class constructor.
Tyrone Tuesday, August 29, 2006
The constructor on class Foo should not be public. Client code can't create instances of this abstract type so this is misleading. Instead of public it should be protected or internal.
Chris Martin Tuesday, August 29, 2006
It's because constructors are NOT inherited. It's in the CLR docs. You call the Bar ctor which only calls the Foo ctor because you typed the object as a Foo. It calls the Foo ctor before it calls the Bar ctor. Therefore, message is null when calling Init() from Foo().
marshall Tuesday, August 29, 2006
just eyeballing the code...

i'm guessing the Foo constructor runs before the Bar constructor so you will get a nullreferenceexception when ToUpper() is called on the string.

AreEyeEkks Tuesday, August 29, 2006
I might be completely wrong about this, but are parent class constructors not called before the contents of the sub-class's constructor?

For instance, as soon as Bar() is called, the consructor for Foo is called which in turn calls Bar's Init() and only after all this is message set to "Hello!". Is my thinking correct here?

This should result in a Null Pointer exception I asume.
Lars Wilhelmsen Tuesday, August 29, 2006
Calling an abstract method from the constructor is a no-no...

--
Lars Wilhelmsen
Alessandro Tuesday, August 29, 2006
Thou shalt never invoke virtual methods in constructors :)

In this particular case, when Bar gets created, it calls the base (Foo) class constructor before doing anything else.
Foo calls Init() in its constructor, which is abstract. This means that Bar.Init() gets called.

Here, you'll be greeted by a nice "null ref exception" :)

BTW: if you wanted to be more evil, you could have defined Foo.Init() as virtual and give it a body. The result would have been the same, but it would have been harder to spot.
James Newton-King Tuesday, August 29, 2006
That's easy: Virtual method call from a constructor.

Init is called before the Bar constructor has run so message will be null.
Doogal Tuesday, August 29, 2006
The overridden Init gets called before Bar's constructor so message is null, bang!

That's why FxCop tells me I'm a bad boy when I do this kind of thing
miroslav Tuesday, August 29, 2006
virtual (in your case, abstract) method is called in base class constructor. This should be avoided.
When the constructor of the Bar class is called, it calls base class (Foo) constructor first, before executing message = "Hello!"; line. The Foo class constructor calls Init() method. This method tries to write the uppercase message, but fails because the message is null. ToUpper() (or any other instance method call) cannot be called on a null string.
Pete Bassett Tuesday, August 29, 2006
The exception is thrown when ToUpper is called because message is null in Bar::Init. This is because the Foo constructor runs before the Bar constructor.

Ido Samuelson Tuesday, August 29, 2006
Very easy.
First thing a Ctor is doing is called it's base class Ctor. Thus Bar Ctor is called first and then its calling Init() which will result in object null reference exception since message was not initialize yet.
Martin Tuesday, August 29, 2006
Bar.Init() runs before Bar's ctor has been called. Member message has not been initialized and is null.
James Higgs Tuesday, August 29, 2006
You're going to get a NullReferenceException because Foo.ctor() is run before Bar.ctor() so when Init is called, message is null.

There's an FxCop rule to warn you about this problem, I believe.
scott Tuesday, August 29, 2006
@Tyrone: good catch.

@ChrisM: Foo's ctor will always be called when I create a Bar (irregardless of my variable type).

To everyone who mentioned "virtual method calls from a constructor" - that is the crux of the problem. The virtual method will execute in the most derived class that overrides the method - and will do so before the ctor of the most derived class executes. Yes, there is an FxCop rule to catch this evil practice.

James Curran Wednesday, August 30, 2006
I should note that in C++, you have the same problem, but a different result. When the Foo ctor calls Init, nothing of the Bar object exists at all, so Foo.Init() is call. But Foo.Init is abstract, so BOOM.
scott Thursday, August 31, 2006
James - yes, I remember that catching me once a long time ago!
Thomas Eyde Thursday, August 31, 2006
I think it's wrong that any code can run in an instanse before it is constructed.

But if it's impossible to avoid this situation, why is virtual calls even possible from the constructor?
Bob Thursday, August 31, 2006
The thing I like most about this series of "how stupid is this language" is how grateful it makes me not to be using C++. I tend to forget just how atrocious it is when I don't get exposed to it, and then a post like this makes me go "hey, there's no calls to the parent constructor... ooh, I remember. Ow!"

So my answer is: there should be a compiler error at the constructor in Foo because it's an abstract class but there's code in it. Abstract classes should only have abstract methods on them, remember.
scott Thursday, August 31, 2006
Bob - Thomas:

I think you both propose that the compiler should prevent this problem. It's such a well known problem that they must have a convincing argument to let it pass.
Comments are now closed.
by K. Scott Allen K.Scott Allen
My Pluralsight Courses
The Podcast!