I've always heard that one should not call overridable methods in constructors, and doing so will cause a violation during compilation. To understand why a build warning is generated, lets look at an example.
First, let me state something that we all should hopefully find reasonable. When instantiating an instance of a class, the first thing that should be called is the class' constructor. So assume we have a class that logs strings to a file:
public class FileLogger
{
private readonly string _logDirectory;
public FileLogger(string logDirectory)
{
if (logDirectory == null)
throw new ArgumentNullException(logDirectory);
_logDirectory = logDirectory;
}
public void Log(string statement)
{
Log.WriteLine(_logDirectory, statement);
}
}Looking at the FileLogger class, we can be sure that any time the Log method is called, the _logDirectory is not null.
public class Service
{
public Service(FileLogger logger)
{
logger.Log("Instantiating Service");
}
}Being a SOLID developer that also likes unit tests, we decide that the FileLogger implementation should really sit behind a generic Logging interface. Lets break apart the interface and implementation.
public interface ILogger
{
void Log(string statement);
}
public class FileLogger : ILogger
{
private readonly string _logDirectory;
public FileLogger(string logDirectory)
{
if (logDirectory == null)
throw new ArgumentNullException(logDirectory);
_logDirectory = logDirectory;
}
public void Log(string statement)
{
Log.WriteLine(_logDirectory, statement);
}
}
public class Service
{
public Service(ILogger logger)
{
logger.Log("Instantiating Service");
}
}Now any time we want to log something in code we can use the ILogger interface and not depend on the concrete FileLogger. This is great for at least two reasons that I will briefly mention.
First, the (Open/Closed Principle)[https://drive.google.com/file/d/0BwhCYaYDn8EgN2M5MTkwM2EtNWFkZC00ZTI3LWFjZTUtNTFhZGZiYmUzODc1/view] says that there should be ways to write code that does not need to be changed when requirements change. Applying this to our example, we have been logging things based on the requirement that logs go to the file system. If the requirement changes, and now logs go to the console, we can create a new ILogger implementation
public class ConsoleLogger : ILogger
{
public void Log(string statement)
{
Console.WriteLine(statement);
}
}
public class Service
{
public Service(ILogger logger)
{
logger.Log("Instantiating Service");
}
}and becuse we are programming to the ILogger interface, the Service class does not need to change, hence the Open/Closed Principle is honored. The other reason is for unit testing. In general, a developer would want to mock any dependencies of the system under test (SUT). If we wanted to unit test the Service class, we would want to mock the logger, and since the dependency on logger is an interface, this is simple to do.
public class ServiceTests
{
[Fact]
public void CanCreateService()
{
var logger = new Mock<ILogger>();
var service = new Service(logger.Object);
logger.Verify(l => l.Log(It.IsAny<string>()), Times.Once());
}
}Okay, we just went on a bit of a tangent, but now that we are convinced that we are doing things right lets take another step toward the problem. Lets assume that all implementations of ILogger are going to share some type of log format that outputs statements regardless of where those statements go (console or file system).
public interface ILogger
{
void Log(string statement);
}
public abstract class LoggerBase : ILogger
{
private readonly DateTime _startTime;
protected LoggerBase()
{
_startTime = DateTime.Now();
}
protected string Format(string statement)
{
return string.Format("{0}, {1}", _startTime, statement);
}
public abstract void Log(string statement);
}
public class ConsoleLogger : LoggerBase
{
public override void Log(string statement)
{
Console.WriteLine(Format(statement));
}
}
public class FileLogger : LoggerBase
{
private readonly string _logDirectory;
private readonly SteamWriter _fileWriter;
public FileLogger(string logDirectory)
{
_logDirectory = logDirectory;
_fileWriter = new StreamWriter(_logDirectory);
}
public override void Log(string statement)
{
_fileWriter.WriteLine(Format(statement));
}
}After looking at the implementation of FileLogger, we decide that we would like to pull out the instatiation of the StreamWriter into a method that is called during initialization.
public class FileLogger : LoggerBase
{
private readonly string _logDirectory;
private SteamWriter _fileWriter;
public FileLogger(string logDirectory)
{
_logDirectory = logDirectory;
Initialize();
}
public void Initialize()
{
_fileWriter = new StreamWriter(_logDirectory);
}
public override void Log(string statement)
{
_fileWriter.WriteLine(Format(statement));
}
}Then, to try and promote enforcing a pattern throughout the code, we decide that we actually want all implementations of ILogger to override an Implementation method, so we refactor the code just a little bit.
public abstract class LoggerBase : ILogger
{
private readonly DateTime _startTime;
protected LoggerBase()
{
_startTime = DateTime.Now();
Initialize();
}
protected abstract void Initialize();
protected string Format(string statement)
{
return string.Format("{0}, {1}", _startTime, statement);
}
public abstract void Log(string statement);
}
public class FileLogger : LoggerBase
{
private readonly string _logDirectory;
private SteamWriter _fileWriter;
public FileLogger(string logDirectory)
{
_logDirectory = logDirectory;
}
protected override void Initialize()
{
_fileWriter = new StreamWriter(_logDirectory);
}
public override void Log(string statement)
{
_fileWriter.WriteLine(Format(statement));
}
}If we compile the latest iteration of the code, we will get the CA2214 build warning. However, even though it is a compile time warning, what we have done will actually cause a run-time error. Remember that a parent class' constructor is always called before the derived class constructor. So when we instantiate a FileLogger, it will call the LoggerBase constructor before it calls the FileLogger constructor. This means that the _logDirectory field will not have been set when the base class constructor calls the overriden Initialize method, and since the _logDirectory field will be null, the Directory.CreateDirectory line will crash the application!