user1511956
user1511956

Reputation: 844

NullReferenceException in C# when the container is initialized

In the C# code listed below, I get a "NullReferenceException" with the error:

"Object reference not set to an instance of an object"

I guess the error is related to the inheritance and/or the template definitions. The list gets initialized, and when debugging I can confirm that the list does not point to NULL. I can't figure out how to do this in another way. (Sorry about the confusing class names / structure). The exception happens here: this.localSMT.doSomething(base.list);

public class VTEST<V>
{
    public List<V> list;
    public LocalSMT<V> localSMT;
    public VTEST()
    {
        list = new List<V>();
    }
}
public class VTEST_FSUB<V> : VTEST<V>
{
    public VTEST_FSUB()
    {
        do_virtual();
    }
    public void do_virtual()
    {
        this.localSMT.doSomething(base.list);
    }
}
public class VTEST_RUN : VTEST_FSUB<int>
{
    public VTEST_RUN()
    {
        localSMT = new VTEST_SUB();
    }
}

public class LocalSMT<V>
{
    public LocalSMT() { }
    public virtual void doSomething(List<V> value) { }
}
public class VTEST_SUB : LocalSMT<int>
{
    public VTEST_SUB(){}
    public override void doSomething(List<int> value) { 
            System.Console.WriteLine("VTEST_SUB VIRTUAL");
    }
}
class Program 
{

    Program() {}

    static void Main(string[] args)
    {
        VTEST_RUN run = new VTEST_RUN();
    }
}

Upvotes: 0

Views: 778

Answers (3)

Jon Skeet
Jon Skeet

Reputation: 1502036

The problem is that the VTEST_FSUB<V> constructor body is executing before the VTEST_RUN constructor body. So when do_virtual is called, localSMT is still null. Then do_virtual tries to call a method on localSMT, hence the exception.

Basically the initialization order for any class in the hierarchy is:

  • Initialize variables which have been declared within an initializer at the point of declaration (any other variables just have the variable type's default value)
  • Chain up to the base class initialization
  • Execute the constructor body

See my article on constructor chaining for more details.

Lessons to learn:

  • Avoid public fields. If you use private fields, it's easy to find every piece of code that reads them and writes to them
  • Ideally, use readonly fields: if you'd passed the value up the constructor chain and set it in the VTEST<V> constructor, you wouldn't have had a problem. (Admittedly readonly fields can still be a pain because of the next point...)
  • Avoid virtual method calls in constructors. In this case that wasn't the problem, but you could easily have had the same issue if do_virtual had been abstract in VTEST_FSUB<V> and overridden to call localSMT.doSomething in VTEST_RUN. It would still have executed before the constructor body had run, which would be surprising. Anything you call within a constructor is operating on a partially-initialized object, which is a precarious situation.
  • Avoid large inheritance hierarchies. They're a pain to work with and reason about.
  • Follow .NET naming conventions! Your code is partly hard to read because it's so unidiomatic. Even when you're just giving sample code, at least follow the capitalization conventions.

Upvotes: 7

Brian
Brian

Reputation: 5119

public class VTEST_RUN : VTEST_FSUB<int>
{
   public VTEST_RUN()
   {
    localSMT = new VTEST_SUB();  // BAD!  localSMT isn't initialized yet!
    }
}

I believe that you have failed to new up one of your objects:

public void do_virtual()
{
   localSMT = new LocalSMT<V>();
   localSMT.doSomething(list);
}

Make sure that when you are trying to use an object that you initialize them! And don't worry too much, this is a very common problem in coding.

Upvotes: 1

Kamil Budziewski
Kamil Budziewski

Reputation: 23107

try:

public void do_virtual()
{
    localSMT=new LocalSMT<V>();
    localSMT.doSomething(list);
}

in public class VTEST_FSUB<V> : VTEST<V>

You are not instatianing localSMT before using, so it's not working.

EDIT: OR

public class VTEST<V>
{
    public List<V> list;
    public LocalSMT<V> localSMT;
    public VTEST()
    {
        list = new List<V>();
        localSMT = new LocalSMT<V>();
    }
}

initialize it in constructor, preferable.

Second solution is cleaner.

Upvotes: 3

Related Questions