I’m just running NDepend against some code that I have written and one of the warnings is Overrides of Method() should call base.Method().
The places this occurs are where I have a base class which has virtual properties and methods with default behaviour but which can be overridden by a class which inherits from the base class and doesn’t call the overridden method.
For example, in the base class I have a property defined like this:
protected virtual char CloseQuote
{
get
{
return '"';
}
}
And then in an inheriting class which uses a different close quote:
protected override char CloseQuote
{
get
{
return ']';
}
}
Not all classes which inherit from the base class use different quote characters hence my initial design.
The alternatives I thought of were have get/set properties in the base class with the defaults set in the constructor:
protected BaseClass()
{
this.CloseQuote = '"';
}
protected char CloseQuote { get; set; }
public InheritingClass()
{
this.CloseQuote = ']';
}
Or make the base class require the values as constructor args:
protected BaseClass(char closeQuote, ...)
{
this.CloseQuote = '"';
}
protected char CloseQuote { get; private set; }
public InheritingClass()
base (closeQuote: ']', ...)
{
}
Should I use virtual in a scenario where the base implementation may be replaced instead of extended or should I opt for one of the alternatives I thought of? If so, which would be preferable and why?
1
It is a valid design decision to have a default implementation of a method in a base-class that should be completely replaced when the method is overridden in a derived class.
Thus, you could regard the warnings from NDepend as a false-positive for your code. I would check the NDepend documentation to see if there is an option or source-code annotation that tells NDepend that not calling the base-class implementation is correct here to get rid of the clutter.
If there is no such option (or it does not work for properties), my preference would be the first alternative (overriding the value of CloseQuote in the derived-class’ constructor).
It’s a strange warning NDepend gives you. Surely not all overridden methods have a need to call the base method, so I think the text Method() should call base.Method() is too strong. It does make it look like you may be doing something wrong, which you clearly are not.
This has nothing to do with using virtual though. Was that supposed to be a different question?
2
The warning is trying to tell you that, since you’ve defined an is-a relationship with the super class, all protected and public behavior of the super class should be reachable from the subclass.
Most often the real reason you want to override a method to replace its behavior completely is a demand for composition. E.g. instead of overriding the method closeQuote, you might want to create an interface ‘QuoteDefinitionProvider’ (or such like) and feed it to the superclass.
Other reasons might be:
- super class should be abstract, but isn’t, so a ‘default’ implementation is given. Solution: make super class abstract.
- subclass implementation can be optimized for the special case. This seems to be a sound reason to completely override the method and not call super.
So, before ignoring this warning, check whether your situation demands composition instead of inheritance. In my professional experience, I’ve seldom found a true need for class inheritance.