4
\$\begingroup\$

Just to quickly explain what this is and what it does: it's a config handler that loads a file that has configuration data in INI format, key=value that sort of format. I then just store it all inside a double string dictionary and take items from the dictionary when I need them.

Questions:

  1. Is my dispose pattern okay? Do I even need to dispose in the first place? I'm still learning about disposing and memory management.
  2. Should I be locking on the dictionary?
  3. Am I okay to use LINQ? I recently got told that LINQ slows down your code. Is there any better way to do this? Is the speed much different?

namespace Pitaya.Root.Core.Config
{
    internal class ConfigHandler : IDisposable
    {
        private static readonly ILogger Logger = LogManager.GetCurrentClassLogger();

        private Dictionary<string, string> _configItems;

        internal ConfigHandler()
        {
            _configItems = new Dictionary<string, string>();
            LoadConfig("/resources/config/config.ini");
        }

        internal void LoadConfig(string configFile)
        {
            try
            {
                if (!File.Exists(configFile) || FileUtilities.IsFileLocked(new FileInfo(configFile)))
                {
                    Logger.Error("Either your config file couldn't be found, or its open somewhere else.");

                    Console.ReadKey(true);
                    Environment.Exit(0);
                }

                _configItems = File.ReadLines(configFile).Where(IsConfigurationLine).Select(line => line.Split('=')).ToDictionary(line => line[0], line => line[1]);
            }
            catch (Exception exception)
            {
                Logger.Fatal("Error while loading config: " + exception.Message);
                Logger.Fatal(exception.StackTrace);

                Console.ReadKey(true);
                Environment.Exit(0);
            }
        }

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

        internal string GetConfigValueByKey(string key)
        {
            if (!_configItems.TryGetValue(key, out var value))
            {
                Logger.Warn("Missing configuration key `" + key + "`");
            }

            return value;
        }

        public void Dispose()
        {
            Dispose(true);
        }

        private void Dispose(bool disposing)
        {
            if (!disposing)
            {
                return;
            }

            _configItems.Clear();
            _configItems = null;
        }
    }
}
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

I recently got told that LINQ slows down your code.

Tell them they are wrong.

Is there any better way to do this?

No. You first code for convenience and pretty code.

Is the speed much different?

It doesn't matter unless you need to optimize it for speed but you do this only for parts that you have and can measure. You don't optimize code that you believe might by slow because someone heard from someone who heard from someone else that something might be slow.


Your ConfigHandler doesn't have to be IDisposable. You are not using any resources and clearing a dictionary isn't doing anything useful.


if (!File.Exists(configFile) || FileUtilities.IsFileLocked(new FileInfo(configFile)))               

You should check only one condition at a time and log a precise message.

"Either your config file couldn't be found, or its open somewhere else."

You are not helping yourself with this message. Each time any of these occurs you'll need to check both cases instead of only one.

\$\endgroup\$
1
  • \$\begingroup\$ LINQ is indeed slower than an equivalent foreach loop doing the same thing, there is a price for creating all those iterators and for the "lazy evaluation". In the vast majority of use cases, however, that extra overhead is negligible, unless you're micro-optimizing. And in this particular case, LINQ is right next to a file read, that's likely going to dominate the runtime rather than any post-processing. \$\endgroup\$
    – Alejandro
    Dec 11, 2017 at 16:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.