Edward B. Edward B. - 3 months ago 25
C# Question

Solving the 'Virtual method call in constructor' issue

I am making a software in c#. I am using an abstract class,

Instruction
, that has these bits of code:

protected Instruction(InstructionSet instructionSet, ExpressionElement newArgument,
bool newDoesUseArgument, int newDefaultArgument, int newCostInBytes, bool newDoesUseRealInstruction) {

//Some stuff

if (DoesUseRealInstruction) {
//The warning appears here.
RealInstruction = GetRealInstruction(instructionSet, Argument);
}
}


and

public virtual Instruction GetRealInstruction(InstructionSet instructionSet, ExpressionElement argument) {
throw new NotImplementedException("Real instruction not implemented. Instruction type: " + GetType());
}


So Resharper tells me that at the marked line I am 'calling a virtual method in constructor' and that this is bad. I understand the thing about the order in which the constructors are called. All overrides of the
GetRealInstruction
method look like this:

public override Instruction GetRealInstruction(InstructionSet instructionSet, ExpressionElement argument) {
return new GoInstruction(instructionSet, argument);
}


So they don't depend on any data in the class; they just return something that depends on the derived type. (so the constructor order doesn't affect them).

So, should I ignore it? I'd rather not; so could anyone show me how could I avoid this warning?

I cannot use delegates neatly because the
GetRealInstruction
method has one more overload.

Answer

I've encountered this issue quite a few times and the best way I've found to properly resolve it is to abstract the virtual method that is being called from the constructor, into a separate class. You would then pass an instance of this new class into the constructor of your original abstract class, with each derived class passing it's own version to the base constructor. It's a bit tricky to explain so I'll give an example, based on yours.

public abstract class Instruction
{
    protected Instruction(InstructionSet instructionSet, ExpressionElement argument, RealInstructionGetter realInstructionGetter)
    {
        if (realInstructionGetter != null)
        {
            RealInstruction = realInstructionGetter.GetRealInstruction(instructionSet, argument);
        }
    }

    public Instruction RealInstruction { get; set; }

    // Abstracted what used to be the virtual method, into it's own class that itself can be inherited from.
    // When doing this I often make them inner/nested classes as they're not usually relevant to any other classes.
    // There's nothing stopping you from making this a standalone class of it's own though.
    protected abstract class RealInstructionGetter
    {
        public abstract Instruction GetRealInstruction(InstructionSet instructionSet, ExpressionElement argument);
    }
}

// A sample derived Instruction class
public class FooInstruction : Instruction
{
    // Passes a concrete instance of a RealInstructorGetter class
    public FooInstruction(InstructionSet instructionSet, ExpressionElement argument) 
        : base(instructionSet, argument, new FooInstructionGetter())
    {
    }

    // Inherits from the nested base class we created above.
    private class FooInstructionGetter : RealInstructionGetter
    {
        public override Instruction GetRealInstruction(InstructionSet instructionSet, ExpressionElement argument)
        {
            // Returns a specific real instruction
            return new FooRealInstuction(instructionSet, argument);
        }
    }
}

// Another sample derived Instruction classs showing how you effictively "override" the RealInstruction that is passed to the base class.
public class BarInstruction : Instruction
{
    public BarInstruction(InstructionSet instructionSet, ExpressionElement argument)
        : base(instructionSet, argument, new BarInstructionGetter())
    {
    }

    private class BarInstructionGetter : RealInstructionGetter
    {
        public override Instruction GetRealInstruction(InstructionSet instructionSet, ExpressionElement argument)
        {
            // We return a different real instruction this time.
            return new BarRealInstuction(instructionSet, argument);
        }
    }
}

In your particular example it does get a little confusing and I started to run out of sensible names, but this is due to the fact you already have a nesting of Instructions within Instructions, i.e. an Instruction has a RealInstruction (or at least optionally does); but as you can see it is still possible to achieve what you want and avoid any virtual member calls from a constructor.

In case that's still not clear, I'll also give an example based on one I used just recently in my own code. In this case I have 2 types of forms, a header form and a message form, both of which inherit from a base form. All forms have fields but each form type has a different mechanism for constructing its fields, so I originally had an abstract method called GetOrderedFields that I called from the base constructor and the method was overriden in each derived form class. This gave me the resharper warning you mention. My solution was the same pattern as above and is as follows

internal abstract class FormInfo
{
    private readonly TmwFormFieldInfo[] _orderedFields;

    protected FormInfo(OrderedFieldReader fieldReader)
    {
        _orderedFields = fieldReader.GetOrderedFields(formType);
    }

    protected abstract class OrderedFieldReader
    {
        public abstract TmwFormFieldInfo[] GetOrderedFields(Type formType);
    }
}

internal sealed class HeaderFormInfo : FormInfo
{
    public HeaderFormInfo()
        : base(new OrderedHeaderFieldReader())
    {
    }

    private sealed class OrderedHeaderFieldReader : OrderedFieldReader
    {
        public override TmwFormFieldInfo[] GetOrderedFields(Type formType)
        {
            // Return the header fields
        }
    }
}

internal class MessageFormInfo : FormInfo
{
    public MessageFormInfo()
        : base(new OrderedMessageFieldReader())
    {
    }

    private sealed class OrderedMessageFieldReader : OrderedFieldReader
    {
        public override TmwFormFieldInfo[] GetOrderedFields(Type formType)
        {
            // Return the message fields
        }
    }
}