Config class - follow-up

Original question: Basic Config Class

One problem I know about is Initialize() not being threadsafe, which I am sorting as we speak, so please disregard that.

```
using log4net;
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;

namespace Apple.Application.Base.Config
{
internal class AppleConfig
{
///
/// List of configuration items
///
private Dictionary _configItems;

///
/// Config file info
///
private readonly FileInfo _configFile;

///
/// Log4net logger.
///
private readonly ILog _log;

///
/// Checks if class has been initialized.
///
private readonly bool _initialized;

///
/// AppleConfig constructor.
///
///
public AppleConfig(string filepath)
{
if (_initialized)
return;

_configItems = new Dictionary();
_configFile = new FileInfo(filepath);
_log = LogManager.GetLogger(typeof(AppleConfig));
_initialized = Initialize();
}

///
/// Initializes AppleConfig
///
private bool Initialize()
{
try
{
foreach (string line in File.ReadLines(_configFile.FullName).Where(IsConfigurationLine))
{
var splittedLine = line.Split('=');

String key = splittedLine[0];
String value = splittedLine[1];

_configItems[key] = value;
}

return true;
}
catch (Exception exception)
{
_log.Error(exception); // assuming overload that takes an exception.
return false;
}
}

///
/// Checks for valid config line
///
/// String to check
///

Merging some of the suggestions you've received so far this is what I think it should look like (I removed the comments for this review):

internal class AppleConfig
{
    private Dictionary _configItems = new Dictionary();

    private readonly string _fileName;

    private readonly ILog _log = LogManager.GetLogger(typeof(AppleConfig));

    public AppleConfig(string fileName)
    {
        _fileName = fileName;

        // first time we want it to work
        if(!LoadConfig())
        {
            throw new Exception("Initalization failed.");
        }
    }

    public string this[string key]
    {
        // this might throw an exception if key not found
        get { return _configItems[key]; }
    }

    public bool IsInitialized
    {
        get { return _configItems != null; }
    }

    public bool LoadConfig()
    {
        try
        {
            _configItems =
                File.ReadLines(_fileName)
                    .Where(IsConfigurationLine)
                    .Select(line => line.Split('='))
                    .ToDictionary(line => line[0], line => line[1]);

            return true;
        }
        catch (Exception exception)
        {
            _log.Error(exception); // assuming overload that takes an exception.
            return false;
        }
    }

    private bool IsConfigurationLine(string line)
    {
        return !line.StartsWith("#") && line.Contains("=");
    }
}

-
You don't need the _isInitialized field, you can check the dictionary if it's not null

-
You don't need the big loop but just a short linq query (by @Konrad Morawski)

-
You don't need the Initialize method and all the crazy logic ;-) but a simple LoadConfig method that you call in the constructor and later if you want to load it again

-
You don't need the FileInfo class but just a simple string for the path

-
There's still a lot that might be improved but it all depends how you are going to use it and whether you need all the improvements at all: will someone else use it? Do you need/want meaningful exceptions (I think it's always hard to implement them if you want them to make it possible to find an error right away).

-
After all I think the IsInitialized property is also not necessary, it doesn't have any usage. The object cannot actually be in an un-initialized state beacause it would be useless then so I would say it can be removed without losing anything. The load method either worked and everything is fine or it didn't and the constuructor throws an exception.

StackExchange Code Review Q#109031, answer score: 4