CODEWITHSUNDEEP
c#fxcop
private_meta
private_meta

Reputation: 561

Suppressing or avoiding Warning CA2214

I have a situation where I create an object called EntryEvent from data I receive. That data has to be parsed. The base event is supposed to kick off parsing of the data that was received through the constructor, and given to the object. The subtype knows how to pars that specific data set. Now, when compiling said code, I get the warning CA2214, that it contains a call chain to a virtual method. While it may be bad to have unforseen consequences, I do not know how to get the required behavior: Parse the received event without having to call an additional "Parse" method from the outside.

The Code in question is:

public abstract class BaseEvent
{
    protected BaseEvent(object stuff)
    {
        this.ParseEvent();
    }

    protected abstract void ParseEvent();
}

public class EntryEvent : BaseEvent
{
    public EntryEvent( object stuff )
        : base( stuff )
    {
    }

    protected override void ParseEvent()
    {
        // Parse event
    }
}

Upvotes: 4

Views: 5174

Answers (1)

Adriano Repetti
Adriano Repetti

Reputation: 67080

According to MSDN (emphasis is mine):

When a virtual method is called, the actual type that executes the method is not selected until run time. When a constructor calls a virtual method, it is possible that the constructor for the instance that invokes the method has not executed.

So in my opinion you have these options (at least):

1) Do not disable that warning but suppress that message for your specific class(es) documenting its intended behavior (assuming you take extra care to deal with such scenario). It's not so bad if it's limited to few classes in a very controlled environment (after all...warnings are not errors and they may be ignored).

2) Remove that virtual method call from base class constructor but leave abstract method declaration there. Developers will have to implement such method and to call it in constructor they will need to mark their classes as sealed. Finally add somewhere in class/method documentation that that method must be called inside their constructor and their class must be sealed to do so. They can forget that call but you may add (for DEBUG builds) a check when properties or methods are accessed (for example forcing, as part of class interface, to set a specific flag). If they forget to set the flag or they forget to call the method then an exception will be raised ("This object has not been built, ParseEvent() must be called in derived classes constructor.").

I don't like this method very much because it adds extra complexity but if your class hierarchy is too big (then you feel you can't use #1) or lazy initialization (described in #3) is not applicable then it may be a working solution. I'd also consider to change design to introduce a factory method that will invoke ParseEvent() for each fully constructed object.

3) Change little bit your design: defer parsing to when it's needed. For example:

public abstract class BaseEvent
{
    public DateTime TimeStamp
    {
        get
        {
            if (_timestamp == null)
                ParseEvent();

            return _timestamp.Value;
        }
        protected set { _timestamp = value; }
    }

    protected BaseEvent(object stuff)
    {
    }

    protected abstract void ParseEvent();

    private DateTime? _timestamp;
}

Last example is only for illustration purposes, you may want to use Lazy<T> to do same task in a more coincise, clear and thread-safe way. Of course in reality you'll have more fields/properties and probably parsing will provide all values in one shot (then you just need a flag, no need for Nullable/special value on each field) This is approach I'd prefer even if it's more verbose.

Upvotes: 4

Related Questions