how to initialize field in base class which should be initialized in subclass?

Question

My base class Car contains field engine which can not be initialized in base class. I can initialize it in subclass only, for example in ElectricCar i can write engine = new ElectricEngine. However I use field in base class. So I have a field which used but not initialized:

public class Car {

    protected Engine engine;

    public void Start() {
        engine.Start();
        // do something else
    }

    public void Stop {
        engine.Stop();
        // do something else
    }

    public void Diagnose() {
        engine.Diagnose();
        // anotherField.Diagnose();
        // oneAnotherField.Diagnose();
    }

}

How to better initialize engine?

Version 1. Field guaranteed to be initialized but with many fields constructor will look ugly. Bug-free but ugly.

public class Car {

    protected Engine engine;

    public Car(Engine engine) {
        this.engine = engine;
    }

    public void Start() {
        engine.Start();
        // do something else
    }

    public void Stop {
        engine.Stop();
        // do something else
    }

    public void Diagnose() {
        engine.Diagnose();
        // anotherField.Diagnose();
        // oneAnotherField.Diagnose();
    }

}

public class ElectricCar : Car {
    public ElectricCar() : base (new ElectricEngine()) {
    }
}

Version 2. Subclasses should remember to initialize the field, having such "contract" with subclasses may introduce bugs (uninitialized field).

public class Car {

    protected Engine engine;

    public Car() {
    }

    public void Start() {
        engine.Start();
        // do something else
    }

    public void Stop {
        engine.Stop();
        // do something else
    }

    public void Diagnose() {
        engine.Diagnose();
        // anotherField.Diagnose();
        // oneAnotherField.Diagnose();
    }

}

public class ElectricCar : Car  {
    public ElectricCar() {
        engine = new ElectricEngine();
    }
}

Version 3. Field guaranteed to be initialized. Constructor is clear. But calling virtual method from constructor (potentially dangerous, not recommended in general).

public class Car {

    protected Engine engine;

    public Car() {
        InitializeEngine();
    }

    protected abstract void InitializeEngine();

    public void Start() {
        engine.Start();
        // do something else
    }

    public void Stop {
        engine.Stop();
        // do something else
    }

    public void Diagnose() {
        engine.Diagnose();
        // anotherField.Diagnose();
        // oneAnotherField.Diagnose();
    }

}

public class ElectricCar : Car  {
    public ElectricCar() {
    }

    protected void override InitializeEngine() {
        engine = new ElectricEngine();
    }
}

So every version has pros and cons. Which version is better? Or probably you can suggest even something else.

The only thing to keep in mind with virtual vs. abstract functions is that in C# and Java, abstract acts more like a [pure virtual function](http://en.wikipedia.org/wiki/Virtual_function#Abstract_classes_and_pure_virtual_functions). The danger with a virtual function in C++ is that a base class provides an implementation, which is why it's unclear or dangerous to call it from the constructor. — Marc Baumbach, Jan 14 '13 at 09:12

Marc Baumbach · Answer 1 · 2013-01-14T08:27:17.137

5

Version 3 is sort of a take on the Template method design pattern. If your base class can't provide a reasonable default implementation, but you require every car to have an engine, delegating the creation to the base class is a very appropriate and safe solution. I would slightly adjust your initialization to be something like this:

protected abstract Engine InitializeEngine();

Then in your constructor for Car:

public Car() {
    engine = InitializeEngine();
}

This will make the contract very clear. Your subclasses simply need to provide an engine and your base class will guarantee that the engine variable is assigned after the constructor is called.

edited Jan 14 '13 at 08:27

answered Jan 14 '13 at 08:06

Marc Baumbach

10,323
2
30
45

1

i've heard many times that "Calling virtual functions from a constructor or destructor is dangerous and should be avoided whenever possible", for example refer to http://stackoverflow.com/questions/962132/calling-virtual-functions-inside-constructors Do you think my question is exception? – Oleg Vazhnev Jan 14 '13 at 08:37
That may be true for virtual functions in C++, but I don't think that's the case in C#. Regardless, IMO, I would actually go with a combined approach of mine and Tigran's solution. I would create a function like `MyEngine` which sets the engine the first time it is called by calling `InitializeEngine`. Then anywhere you reference the private variable, replace it with a call to `MyEngine`. This delegates creation to subclasses and will create the engine when it's needed rather than when the car is constructed. – Marc Baumbach Jan 14 '13 at 08:41
it's nice to have engine in car right after it constructed, i think we shouldn't use lazy initialization everywhere. simple approach should be prefered when possible. – Oleg Vazhnev Jan 14 '13 at 08:45
The other reason I like this delegate creation over passing something into the constructor is from an OO point of view, the particular car is in full control of the engine it creates/uses. For example, if you passed in a GasolineEngine into your ElectricCar, that car may not start. :) In addition to that, a particular subclass of car may have a closer relationship to the type of engine it uses and therefore use methods on that engine that aren't available in the Engine superclass. – Marc Baumbach Jan 14 '13 at 08:58
The reason to avoid this pattern is that at the point at which the `InitializeEngine()` call is made in the base class, _the constructor for your subclass has not yet run_. If your abstract method implementation relies on any side-effects of that constructor (for example, any fields it initializes) then it won't work. This is often not that obvious to subsequent maintainers of your code, especially once your subclass has grown in size. – Alastair Maw Nov 29 '16 at 18:36

Tigran · Answer 2 · 2013-01-14T08:36:56.100

2

Another option can be something like:

public class Car {

    private Engine engine; //PRIVATE  

    protected Engine MyEngine {   //PROTECTED PROPERTY
        get {
            if(engine == null) 
               engine = new Engine(); 
            return engine;
        }
    }
}

In this way the caller will be safe that will use always initialized member, as it checked inside protected property it can access only, as field is private.

edited Jan 14 '13 at 08:36

answered Jan 14 '13 at 08:06

Tigran

61,654
8
86
123

This doesn't let different child classes specify a different engine, which looks to be a requirement from the question. (`"which can not be initialized in base class"`) – Rawling Jan 14 '13 at 08:33
@Rawling: if you want to be able to override in child of *Car*, the property can become *virtual *. – Tigran Jan 14 '13 at 08:39
This would work if instead of creating a `new Engine()` explicitly, you would call an `abstract` function, ie `protected abstract Engine GetEngine()`, to set the engine, which you could then implement in child classes. – w5l Jan 14 '13 at 08:58

score 1 · Answer 3 · answered Jan 14 '13 at 08:32

1

I'd vote for option 1. You're clearly stating in the constructor that every Car must have an Engine, BrakingSystem, ECU, etc. You also know that these have been created before the Car. If you delay the creation of them until first access and there is an issue creating them, then it will be more difficult to handle the exception appropriately.

answered Jan 14 '13 at 08:32

SimonC

6,590
1
23
40

i also like Version 1 because it straightforward and bug-free, but constructor, and especially "base constructor call" may look ugly. this can be partly solved by introducing new class to aggregate too much parameters. – Oleg Vazhnev Jan 14 '13 at 08:41

Hamlet Hakobyan · Answer 4 · 2013-01-14T10:49:36.673

Then use property instead of field for Engine, because non-private fields are very hard to debug.

About design For first, you must declare Car as abstract and use IEngine behavior instead of Engine class. Then for any concrete car (i.e. subclass) you can choose appropriate type of injection (by constructor, by property, ...).

public interface IEngine
{
     void Start();
     void Stop();
     void Diagnose();
}


public abstract class Car
{
    protected Car(IEngine engine)
    {
         Engine = engine;
    }

    protected IEngine Engine {get; set;}

    public void Start() {
        engine.Start();
        // do something else
    }

    public void Stop() {
        engine.Stop();
        // do something else
    }

    public void Diagnose() {
        engine.Diagnose();
        // anotherField.Diagnose();
        // oneAnotherField.Diagnose();
    }
}

public class ConcreteCar : Car
{
    public ConcreteCar(IEngine engine):base(engine)  // injection by constructor
    {
    }

    ...
}

Usage:

Car concreteCar = new ConcreteCar(new ConcreteEngine());

EDIT

You can force derived classes to initialize engine. See updated example.

this is almost the same to `Version 2`. you pass `engine` in constructor. But when someone else design a new car he might forgot to initialize `engine` field. So `Version 1` is less error-phrone than this one. — Oleg Vazhnev, Jan 14 '13 at 08:49

how to initialize field in base class which should be initialized in subclass?

4 Answers4