How to Avoid Calling Viritual Methods from a Base Constructor

Question

I have an abstract class in a library. I'm trying to make it as easy as possible to properly implement a derivation of this class. The trouble is that I need to initialize the object in a three-step process: grab a file, do a few intermediate steps, and then work with the file. The first and last step are particular to the derived class. Here's a stripped-down example.

abstract class Base
{
    // grabs a resource file specified by the implementing class
    protected abstract void InitilaizationStep1();

    // performs some simple-but-subtle boilerplate stuff
    private void InitilaizationStep2() { return; }

    // works with the resource file
    protected abstract void InitilaizationStep3();

    protected Base()
    {
        InitilaizationStep1();
        InitilaizationStep2();
        InitilaizationStep3();
    }
}

The trouble, of course, is the virtual method call in the constructor. I'm afraid that the consumer of the library will find themselves constrained when using the class if they can't count on the derived class being fully initialized.

I could pull the logic out of the constructor into a protected Initialize() method, but then the implementer might call Step1() and Step3() directly instead of calling Initialize(). The crux of the issue is that there would be no obvious error if Step2() is skipped; just terrible performance in certain situations.

I feel like either way there is a serious and non-obvious "gotcha" that future users of the library will have to work around. Is there some other design I should be using to achieve this kind of initialization?

I can provide more details if necessary; I was just trying to provide the simplest example that expressed the problem.

Answer 1

That's way too much to place in the constructor of any class, much less of a base class. I suggest you factor that out into a separate Initialize method.

Answer 2

Edit: I answered this for C++ for some reason. Sorry. For C# I recommend against a Create() method - use the constructor and make sure the objects stays in a valid state from the start. C# allows virtual calls from the constructor, and it's OK to use them if you carefully document their expected function and pre- and post-conditions. I inferred C++ the first time through because it doesn't allow virtual calls from the constructor.

Make the individual initialization functions private. The can be both private and virtual. Then offer a public, non-virtual Initialize() function that calls them in the correct order.

If you want to make sure everything happens as the object is created, make the constructor protected and use a static Create() function in your classes that calls Initialize() before returning the newly created object.

Answer 3

In lots of cases, initialization stuff involves assigning some properties. It's possible to make those properties themselves abstract and have derived class override them and return some value instead of passing the value to the base constructor to set. Of course, whether this idea is applicable depends on the nature of your specific class. Anyway, having that much code in the constructor is smelly.

Answer 4

At first sight, I would suggest to move this kind of logic to the methods relying on this initialization. Something like

public class Base
{
   private void Initialize()
   {
      // do whatever necessary to initialize
   }

   public void UseMe()
   {
      if (!_initialized) Initialize();
      // do work
   }
}

Answer 5

I would consider creating an abstract factory that is responsible for instantiating and initializing instances of your derived classes using a template method for initialization.

As an example:

public abstract class Widget
{
    protected abstract void InitializeStep1();
    protected abstract void InitializeStep2();
    protected abstract void InitializeStep3();

    protected internal void Initialize()
    {
        InitializeStep1();
        InitializeStep2();
        InitializeStep3();
    }

    protected Widget() { }
}

public static class WidgetFactory
{
    public static CreateWidget<T>() where T : Widget, new()
    {
        T newWidget = new T();
        newWidget.Initialize();
        return newWidget;
    }
}

// consumer code...
var someWidget = WidgetFactory.CreateWidget<DerivedWidget>();

This factory code could be improved dramatically - especially if you are willing to use an IoC container to handle this responsibility...

If you don't have control over the derived classes, you may not be able to prevent them from offering a public constructor that can be called - but at least you can establish a usage pattern that consumers could adhere to.

It's not always possible to prevent users of you classes from shooting themselves in the foot - but, you can provide infrastructure to help consumers use your code correctly when they familiarize themselves with the design.

Answer 6

Since step 1 "grabs a file", it might be good to have Initialize(IBaseFile) and skip step 1. This way the consumer can get the file however they please - since it is abstract anyways. You can still offer a 'StepOneGetFile()' as abstract that returns the file, so they could implement it that way if they choose.

DerivedClass foo = DerivedClass();
foo.Initialize(StepOneGetFile('filepath'));
foo.DoWork();

Answer 7

You could employ the following trick to make sure that initialization is performed in the correct order. Presumably, you have some other methods (DoActualWork) implemented in the base class, that rely on the initialization.

abstract class Base
{
    private bool _initialized;

    protected abstract void InitilaizationStep1();
    private void InitilaizationStep2() { return; }
    protected abstract void InitilaizationStep3();

    protected Initialize()
    {
        // it is safe to call virtual methods here
        InitilaizationStep1();
        InitilaizationStep2();
        InitilaizationStep3();

        // mark the object as initialized correctly
        _initialized = true;
    }

    public void DoActualWork()
    {
        if (!_initialized) Initialize();
        Console.WriteLine("We are certainly initialized now");
    }
}

Answer 8

I would do something like this below:

public abstract class FileWorkProcessBase
{

    public FileWorkProcessBase()
    {
    }


    public void BeginProcessingFile()
    {

        GrabFile();

        InitalizeFile();

        ProcesFile();
    }

    protected abstract void GrabFile();

    private void InitalizeFile(){
        //Do common file initialization here...
    }

    protected abstract void ProcesFile();

}


public class ExampleFileWorkProcess : FileWorkProcessBase
{
    protected override void GrabFile()
    {
        throw new NotImplementedException();
    }

    protected override void ProcesFile()
    {
        throw new NotImplementedException();
    }
}

Answer 9

I wouldn't do this. I generally find that doing any "real" work in a constructor ends up being a bad idea down the road.

At the minimum, have a separate method to load the data from a file. You could make an argument to take it a step further and have a separate object responsible for building one of your objects from file, separating the concerns of "loading from disk" and the in-memory operations on the object.