Making sure initialization code is executed before other code using Lazy Initialization The Problem

Recently I was told to refactor i.e. move some initialization logic from the constructor. I came up with something similar to the posted code.
In actual there are bunch of loader classes inheriting from ILoader and doing all kind of stuff (TPL, DB access,XML parsing, CSV Writing)
One of our UBER Developers reviewed my code and give some really appreciative comments particularly with the line of code marked with //&lt;--:


  Wow – I have seen a lot but that would make the
  http://thedailywtf.com/ for sure. HOW can you think of having a Getter
  method have a initialization SIDE EFFECT??


What is your opinion guys. What should I've done to avoid such appreciative comments?

The Solution

using System;

public class Program
{
    public static void Main()
    {
        var obj = new TestLoader("testing");
        Console.WriteLine("Calling Run() first time....");
        obj.Run();
        Console.WriteLine("Calling Run() again...");
        obj.Run();
        Console.WriteLine("Calling SomeOtherMethod()...");
        obj.SomeOtherMethod();
    }
}

//Class to make sure initialize will only be called once
public abstract class DataInitializer&lt;T&gt;
{
    public bool IsInitialized { get { return _isInitialized.Value; } }//&lt;--
    readonly Lazy&lt;bool&gt; _isInitialized;
    protected DataInitializer(T param)
    {
        Console.WriteLine("Inside DataInitializer constructor");
        _isInitialized = new Lazy&lt;bool&gt;(Initialize);
    }
    protected abstract bool Initialize();
}

//Some loader class
public class TestLoader :DataInitializer&lt;string&gt;, ILoader
{
    public TestLoader(string test):base(test)
    {
    }

    protected override bool Initialize()
    {
        Console.WriteLine("Inside Initialize()");
        return true;
    }

    public void Run()
    {
        if(IsInitialized)
        {
            Console.WriteLine("Inside Run()");
        }
    }


    public void SomeOtherMethod()
    {
        if(IsInitialized)
        {
            Console.WriteLine("Inside SomeOtherMethod()");
        }
    }

}

interface ILoader
{
    void Run();
    void SomeOtherMethod();
}
Your code does feel like a misuse of lazy evaluation. Some alternative approaches to consider:


MakeInitialize method public and throw if it was not called:

public override void Initialize()
{
    //do stuff
    _isInitialized = true;
}

public void Run()
{
    if(!_isInitialized) throw new InvalidOperationException();
    //do stuff
}


IMHO, it is all-around best option, as it allows you to control when to do initialization. 
Call initialization automatically on every Run call:

protected override void Initialize()
{
    if (_isInitialized) return;
    //do stuff
    _isInitialized = true;
}

public void Run()
{
    Initialize();
    //do stuff
}


This approach is good in some cases. For example, when you have an unstable network connection, it makes sense to check the connection before every method call, and reconnect if needed.
Use Lazy objects to initialize actual services:

public CsvWriter CsvWriter { get { return _csvWriter.Value; } }
readonly Lazy&lt;CsvWriter&gt; _csvWriter;

public void Run()
{
    CswWriter.Write(...)
}


I'm not a fan of lazy initialization, but it is fine, if you use it to instantiate one or two actual services. Laziness is justified in scenarios when there are high chances that Run method will not be called at all by your software.


P.S. Note, that Lazy is thread-safe, while options #1 and #2 are not and require synchronization if thread-safety is required.