How to make my Initialize class method only called once, what would be the best approach?

advertisements

I'm currently using Unity IoC Container and here is my AppConfig class. As you can see the Initialize method should be called only once and I have used double lock checking to ensure that.

What would be the best way to implement achieve this if my approach is not the best way?

public interface IAppConfig
{
    /// <summary>
    /// Gets the admin username.
    /// </summary>
    /// <value>The admin username.</value>
    string AdminUsername { get; }

    /// <summary>
    /// Gets the admin password.
    /// </summary>
    /// <value>The admin password.</value>
    string AdminPassword { get; }

    /// <summary>
    /// Initializes this instance.
    /// </summary>
    void Initialize();
}

/// <summary>
/// A singleton App config which helps reading from web.config
/// its lifetime is controlled by Unity.
/// </summary>
public class AppConfig : IAppConfig
{
    #region Fields

    /// <summary>
    /// the injectable config manager
    /// </summary>
    private readonly IConfigManager _configManager;

    private readonly ILogger _logger;

    private static readonly object LockObject = new object();

    private static bool _initialized = false;

    #endregion

    #region Constructors

    /// <summary>
    /// Initializes a new instance of the <see cref="AppConfig"/> class.
    /// </summary>
    public AppConfig(IConfigManager configManager, ILogger logger)
    {
        this._configManager = configManager;
        this._logger = logger;
    }

    #endregion

    #region Properties

    /// <summary>
    /// Gets the admin username.
    /// </summary>
    /// <value>The admin username.</value>
    public string AdminUsername { get; private set; }

    /// <summary>
    /// Gets the admin password.
    /// </summary>
    /// <value>The admin password.</value>
    public string AdminPassword { get; private set; }

    #endregion

    #region Methods

    public void Initialize()
    {
        if (_initialized)
        {
            throw new ApplicationException("Initialize method should be called only once");
        }

        lock(LockObject)
        {
            if (_initialized) return;

            var adminUserNameSetting = _configManager.AppSettings[ConfigKeys.AdminUsername];

            if (adminUserNameSetting == null)
            {
                throw new ApplicationException("AdminUsername key not found");
            }

            this.AdminUsername = adminUserNameSetting.Value;

            if (String.IsNullOrWhiteSpace(this.AdminUsername))
            {
                _logger.LogError("AdminUsername not found");
            }

            // log
            var adminPasswordSetting = _configManager.AppSettings[ConfigKeys.AdminPassword];

            if (adminPasswordSetting == null)
            {
                throw new ApplicationException("AdminPassword key not found");
            }

            this.AdminPassword = adminPasswordSetting.Value;

            if (String.IsNullOrWhiteSpace(this.AdminPassword))
            {
                _logger.LogError("AdminPassword not found");
            }
            _initialized = true;
        }
    }

    #endregion
}

In the Unity, I'm using the below code:

// IAppConfig
container.RegisterType<IAppConfig, AppConfig>(new ContainerControlledLifetimeManager(),
                                              new InjectionConstructor(configManager,
                                                                       logger));
var appConfig = container.Resolve<IAppConfig>();
appConfig.Initialize();


I think an Initalize() method tastes more like an implementation issue. And that means that maybe it shouldn't be in the interface at all.

Initializing an instance is best left to the constructor.

If you really need a delayed Initialize then you solution with a bool and a lock seems OK.