Simple Configuration System

Question

I have created a simple ConfigManager in C# that loads each line of a file. If the line contains = then it gets the before = (the key) and the after = (the value).

For example, my config file would look something like this:

# Comment here
MyConfigKey1=MyValue1
MyConfigKey2=MyValue2
MyConfigKey3=MyValue3

Obviously my config file wouldn't be that, but that's the layout of it. If the file is unreadable or is not found then the defaults are used.

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;

namespace Sirius.Core.Config
{
    internal sealed class ConfigManager
    {
        private readonly Dictionary<string, string> _configItems;

        public ConfigManager(string configFile)
        {
            _configItems = new Dictionary<string, string>();

            var logManager = Sirius.GetServer().GetLogManager();
            var stopwatch = Stopwatch.StartNew();

            try
            {
                if (File.Exists(configFile) && Sirius.GetServer().GetUtility().IsFileLocked(new FileInfo(configFile)))
                {
                    _configItems = File.ReadLines(configFile)
                        .Where(IsConfigurationLine)
                        .Select(line => line.Split('='))
                        .ToDictionary(line => line[0], line => line[1]);

                }
                else
                {
                    _configItems.Add("database.host", "localhost");
                    _configItems.Add("database.username", "root");
                    _configItems.Add("database.password", "");
                    _configItems.Add("database.name", "database");
                    _configItems.Add("database.port", "3306");
                    _configItems.Add("database.max_connections", "10000");
                    _configItems.Add("sockets.port", "30000");
                    _configItems.Add("sockets.backlog", "50");

                    logManager.Warn("Using the default configuration.");
                }
            }
            catch (Exception exception)
            {
                logManager.Error(exception.Message, exception);
            }
            finally
            {
                logManager.Information("Loaded Config Data [" + stopwatch.ElapsedMilliseconds + "ms]");
                stopwatch.Stop();
            }
        }

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

        public string GetConfigElement(string key)
        {
            string value;

            if (!_configItems.TryGetValue(key, out value))
            {
                Sirius.GetServer().GetLogManager().Error("Missing configuration key `" + key + "`");
            }

            return value;
        }
    }
}

Answer 1

The ConfigManager needs some clean-up because to my taste it's too tigthly coupled to the Sirius server and thus it's not testable.

First, make the constructor simpler. Constructors should not do any heavy job. Ideally the should only initialize fields or properties.

Move its logic into a new method like LoadFromOrDefault so that you know that it either loads the configuration from the specified file or gives you a default one that we move into a new DefaultSettings method.

The new LoadFromOrDefault not requries you to specify the file-name, a Func to determine whether the file is locked and a logger.

internal sealed class ConfigManager : IEnumerable<KeyValuePair<string, string>>
{
    private readonly Dictionary<string, string> _settings;
    private readonly ILogger _logger;

    public ConfigManager(IDictionary<string, string> settings, ILogger logger)
    {
        _settings = new Dictionary<string, string>(settings);
        _logger = logger;       
    }

    public static IDictionary<string, string> DefaultSettings() => new Dictionary<string, string>
    {
        { "database.host", "localhost" },
        // ..
    };

    public static ConfigManager  LoadFlomOrDefault(string fileName, Func<string, bool> isLocked, ILogger logger)
    {
        var stopwatch = Stopwatch.StartNew();

        try
        {
            if (File.Exists(fileName) && isLocked(fileName))
            {
                var settings = File.ReadLines(fileName)
                    .Where(IsConfigurationLine)
                    .Select(line => line.Split('='))
                    .ToDictionary(line => line[0], line => line[1]);
                return new ConfigManager(settings, logger);
            }
            else
            {
                logger.Warn("Using the default configuration.");
                return new ConfigManager(DefaultSettings(), logger);
            }
        }
        catch (Exception exception)
        {
            logger.Error(exception.Message, exception);
            return null;
        }
        finally
        {
            logger.Information("Loaded Config Data [" + stopwatch.ElapsedMilliseconds + "ms]");
            stopwatch.Stop();
        }
    }

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

    public string GetConfigElement(string key)
    {
        string value;

        if (!_settings.TryGetValue(key, out value))
        {
            _logger.Error("Missing configuration key `" + key + "`");
        }

        return value;
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        return GetEnumerator();
    }

    public IEnumerator<KeyValuePair<string, string>> GetEnumerator()
    {
        return _settings.GetEnumerator();
    }
}

You can use it like this:

var config = ConfigManager.LoadFromOrDefault(
    "myconfig.ini", 
    fileName => Sirius.GetServer().GetUtility().IsFileLocked(new FileInfo(configFile)), 
    Sirius.GetServer().GetLogManager()
);

And agian, you can now test it without the Sirius server.

---

Desclimer: This is just a rough idea and not ready-to-use code so it might contain some small bugs or not entirely satisfy your needs yet.

Answer 2

There's a bug in this method.

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

The following input from a file will bring cause the check to return true, likely bringing the rest of the code to its knees.

=SomeValue

It doesn't start with a comment and contains an equals sign, but isn't a valid configuration line.