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

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());
  }
}

posted on Monday, August 28, 2006 9:56 PM by scott

Comments

Monday, August 28, 2006 7:31 PM by Grim

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

NullReferenceException

Because the base class constructor executes before the child class constructor.
Monday, August 28, 2006 8:52 PM by Tyrone

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

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.
Monday, August 28, 2006 9:11 PM by Chris Martin

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

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().
Monday, August 28, 2006 9:21 PM by marshall

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

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.

Monday, August 28, 2006 9:44 PM by AreEyeEkks

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

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.
Tuesday, August 29, 2006 12:12 AM by Lars Wilhelmsen

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

Calling an abstract method from the constructor is a no-no...

--
Lars Wilhelmsen
Tuesday, August 29, 2006 12:32 AM by Alessandro

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

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.
Tuesday, August 29, 2006 12:44 AM by James Newton-King

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

That's easy: Virtual method call from a constructor.

Init is called before the Bar constructor has run so message will be null.
Tuesday, August 29, 2006 1:12 AM by Doogal

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

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
Tuesday, August 29, 2006 1:33 AM by miroslav

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

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.
Tuesday, August 29, 2006 2:05 AM by Pete Bassett

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

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.

Tuesday, August 29, 2006 3:36 AM by Ido Samuelson

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

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.
Tuesday, August 29, 2006 4:23 AM by Martin

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

Bar.Init() runs before Bar's ctor has been called. Member message has not been initialized and is null.
Tuesday, August 29, 2006 5:04 AM by James Higgs

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

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.
Tuesday, August 29, 2006 7:03 AM by scott

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

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

Tuesday, August 29, 2006 7:19 PM by Christopher Steen

# Link Listing - August 29, 2006

Hitting the WPF Curve (a.k.a. Falling off the WPF Cliff) [Via: smguest ] Codesmith Exception Template...
Wednesday, August 30, 2006 2:54 PM by James Curran

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

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.
Wednesday, August 30, 2006 6:59 PM by scott

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

James - yes, I remember that catching me once a long time ago!
Thursday, August 31, 2006 12:00 AM by Thomas Eyde

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

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?
Thursday, August 31, 2006 2:39 AM by Bob

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

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.
Thursday, August 31, 2006 10:32 AM by scott

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

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.