I know there are similar questions already asked here on SO, but specifically my question deals with scenarios involving setting a readonly
field by calling a virtual member in an abstract class's constructor.
Consider the following abstract class:
public abstract class FooBase
{
private readonly IDictionary<string,object> _readonlyCache;
protected abstract IDictionary<string,object> SetCache();
protected FooBase()
{
_readonlyCache = SetCache();
}
}
Questions:
1) Is this just flat-out bad design?
2) Is there a better design?
I'm aware that you could declare implementers of FooBase
as sealed
and that will insure that only the correct implementation of SetCache()
is called. What I don't like about that is there is no way to enforce that implementers must be marked as sealed
. Any suggestions are very welcome.
It's definitely something to be avoided if possible - calling virtual methods within a constructor is always a bit smelly, as you'll be executing code before the subclass gets to perform initialization - its constructor body won't have executed. This is true regardless of whether the subclass is sealed or not; you're in a fundamentally nasty situation.
You might want to consider making the subclass constructor pass the cache up to the constructor:
public abstract class FooBase
{
private readonly IDictionary<string,object> _readonlyCache;
protected FooBase(IDictionary<string,object> cache)
{
_readonlyCache = cache;
}
}
That way the direct subclass gets to decide what to do - it might be abstract and take a cache from a further-derived class, for example, or it might construct its own.
See more on this question at Stackoverflow