5
\$\begingroup\$

For my game I decided to incorporate .ini Initialization Files to store game data. Items like: Weapons, Armor, Potions, Enemies, Actors, Loot Drops, and so much more. All of these things can be easily expressed as simple .ini files with the header-based schema, as I will show later. This design decision also came about with the desire for performance. Naturally I could have used XML or JSON, but I feel as though it would have been overkill, and slower than my own custom parser which does as little as possible.

I wanted to show you guys my implementation of a .ini parser, to see if you could suggest any changes, or things that could possibly increase the performance of it. Out of everything, performance is my number one concern.

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

namespace GrimoireEngine.Framework.Utilities
{
    public class InitializationFile
    {
        public Dictionary<string, Dictionary<string, string>> Data { get; set; }

        public const string Extension = ".ini";

        private string _fileName;

        public string FileName
        {
            get
            {
                return _fileName;
            }
        }

        private string _source;

        public string Source
        {
            get { return _source; }
            set
            {
                if (value.EndsWith(Extension))
                {
                    this._source = value;
                }
                else
                {
                    throw new FileLoadException(Path.GetExtension(value) + " is not a valid " + Extension + " file Extension!");
                }
            }
        }

        private string _mountedHeader;

        public string MountedHeader
        {
            get
            {
                return _mountedHeader;
            }
            set
            {
                if (HasHeader(value))
                {
                    this._mountedHeader = value;
                }
                else
                {
                    throw new KeyNotFoundException("Header \"" + value + "\"" + " does not exist!");
                }
            }
        }

        public int HeaderCount
        {
            get
            {
                return Data.Count;
            }
        }

        public int KeyValueCount
        {
            get
            {
                int count = 0;
                foreach (KeyValuePair<string, Dictionary<string, string>> header in Data)
                {
                    foreach (KeyValuePair<string, string> keyValuePairs in header.Value)
                    {
                        count++;
                    }
                }
                return count;
            }
        }

        public InitializationFile(string path, bool loadImmediate = false)
        {
            this.Data = new Dictionary<string, Dictionary<string, string>>();
            this.Source = path;
            if (loadImmediate)
            {
                Import(path);
            }
        }

        public InitializationFile()
        {
            this.Data = new Dictionary<string, Dictionary<string, string>>();
        }

        public string[] GetHeaders()
        {
            return this.Data.Keys.ToArray();
        }

        public string[] GetKeys(string header)
        {
            if (HasHeader(header))
            {
                return this.Data[header].Keys.ToArray();
            }
            throw new KeyNotFoundException("Header \"" + header + "\"" + " does not exist!");
        }

        public bool HasHeader(string header)
        {
            return this.Data.ContainsKey(header);
        }

        public bool HasHeaderMounted()
        {
            return this.MountedHeader != null && this.Data.ContainsKey(this.MountedHeader);
        }

        public bool HasHeaderMounted(string header)
        {
            return this.MountedHeader == header;
        }

        public void UnmountHeader()
        {
            this._mountedHeader = null;
        }

        public bool HasKey(string header, string key)
        {
            if (HasHeader(header))
            {
                if (this.Data[header].ContainsKey(key))
                {
                    return true;
                }
                return false;
            }
            return false;
        }

        public string GetValue(string header, string key)
        {
            if (HasHeader(header))
            {
                if (HasKey(header, key))
                {
                    return this.Data[header][key];
                }
                throw new KeyNotFoundException();
            }
            throw new KeyNotFoundException();
        }

        public string GetValue(string key)
        {
            if (this.HasHeaderMounted())
            {
                return GetValue(this.MountedHeader, key);
            }
            throw new Exception("No Header is Mounted!");
        }

        public string[] GetKeys()
        {
            if (this.HasHeaderMounted())
            {
                return GetKeys(this.MountedHeader);
            }
            throw new Exception("No Header is Mounted!");
        }

        public void MountHeader(string headerName)
        {
            this.MountedHeader = headerName;
        }

        public void AddHeader(string headerName, bool mountHeader = false)
        {
            if (!this.Data.ContainsKey(headerName))
            {
                this.Data.Add(headerName, new Dictionary<string, string>());
                if (mountHeader)
                {
                    this.MountedHeader = headerName;
                }
            }
        }

        public void AddData(string headerName, string key, string value)
        {
            if (HasHeader(headerName))
            {
                this.Data[headerName].Add(key, value);
            }
            else
            {
                AddHeader(headerName);
                AddData(headerName, key, value); // Recursive
            }
        }

        public void AddData(string key, string value)
        {
            if (HasHeaderMounted())
            {
                AddData(this.MountedHeader, key, value);
            }
        }

        public void SetData(string header, string key, string value)
        {
            if (HasHeader(header))
            {
                if (HasKey(header, key))
                {
                    this.Data[header][key] = value;
                }
            }
        }

        public void SetData(string key, string value)
        {
            if (HasHeaderMounted())
            {
                SetData(this.MountedHeader, key, value);
            }
        }

        public void Export(string filePath)
        {
            if (!filePath.EndsWith(Extension))
            {
                filePath = Path.ChangeExtension(filePath, Extension);
            }
            using (StreamWriter writetext = new StreamWriter(filePath))
            {
                foreach (KeyValuePair<string, Dictionary<string, string>> header in Data)
                {
                    writetext.WriteLine("[" + header.Key + "]");
                    foreach (KeyValuePair<string, string> keyValuePairs in header.Value)
                    {
                        writetext.WriteLine(keyValuePairs.Key + "=" + keyValuePairs.Value);
                    }
                }
            }
            UnmountHeader();
        }

        public void Export()
        {
            Export(this.Source);
        }

        public void Import(string path)
        {
            if (File.Exists(path))
            {
                this.Source = path;
                const int bufferSize = 128;
                using (FileStream fileStream = File.OpenRead(this.Source))
                {
                    using (StreamReader streamReader = new StreamReader(fileStream, Encoding.UTF8, true, bufferSize))
                    {
                        string line;
                        while ((line = streamReader.ReadLine()) != null)
                        {
                            ParseLine(line, this);
                        }
                    }
                }
                this._fileName = Path.GetFileName(path);
                UnmountHeader();
            }
            else
            {
                throw new FileNotFoundException("Could not locate: " + path);
            }
        }


        public void Import()
        {
            if (this.Source != null)
            {
                Import(this.Source);
            }
        }

        private static string ParseHeader(string header)
        {
            header = header.Substring(1, header.Length - 2);
            if (HasWhiteSpaceFast(header))
            {
                header = header.Trim();
            }
            return header;
        }

        private static KeyValuePair<string, string> ParseKeyValuePair(string keyValuePair)
        {
            // Normal KeyValue Pair
            string[] split = keyValuePair.Split('=');
            string key = split[0];
            string value = split[1];
            if (HasWhiteSpaceFast(key))
            {
                key = key.Trim();
            }
            if (HasWhiteSpaceFast(value))
            {
                value = value.Trim();
            }
            return new KeyValuePair<string, string>(key, value);
        }

        public void ParseLines(string[] data)
        {
            for (int i = 0; i < data.Length; i++)
            {
                ParseLine(data[i], this);
            }
        }

        private static void ParseLine(string line, InitializationFile file)
        {
            if (line.Length == 0)
            {
                /**
                 * The line was blank. In this situation
                 * we simply skip to the next iteration
                 * of the parsing algorithm.
                 */
                return;
            }
            char firstChar = line[0];
            switch (firstChar)
            {
                case '[': // A Header/Section
                          /**
                           * A new Header was detected. Create
                           * a new collection to hold all of its 
                           * keys and values.
                           */
                    file.AddHeader(ParseHeader(line), true);
                    break;
                case '#': // Stacked
                          /**
                           * Intentionally Ignore Comments
                           * for performance reasons.
                           */
                case ';': // Stacked
                          /**
                           * Intentionally Ignore Comments
                           * for performance reasons.
                           */
                    break;
                default: // We default to a normal KeyValuePair
                    if (ContainsFast(line, '='))
                    {
                        KeyValuePair<string, string> keyValuePair = ParseKeyValuePair(line);
                        file.AddData(file.MountedHeader, keyValuePair.Key, keyValuePair.Value);
                    }
                    break;
            }
        }

        private static bool HasWhiteSpaceFast(string data)
        {
            return data[0] == 0x20 || data[data.Length - 1] == 0x20;
        }

        private static bool ContainsFast(string data, char delimiter)
        {
            for (int i = 0; i < data.Length; i++)
            {
                if (data[i] == delimiter)
                {
                    return true;
                }
            }
            return false;
        }
    }
}

And a brief usage example:

using System;
using System.Windows;
using GrimoireEngine.Framework.Utilities;

namespace GrimoireDevelopmentKit
{
    /// <summary>
    /// Interaction logic for App.xaml
    /// </summary>
    public partial class App : Application
    {
        protected override void OnStartup(StartupEventArgs e)
        {
            InitializationFile file = new InitializationFile("C:/Users/Krythic/Desktop/GameItem_01.ini", true);
            foreach (string header in file.GetHeaders())
            {
                Console.WriteLine("[" + header + "]");
                foreach (string key in file.GetKeys(header))
                {
                    Console.WriteLine( key + " = " + file.GetValue(header, key));
                }
            }

        }
    }
}

And here is a mockup game item .ini for testing purposes.

[Information]
Name = "Imbued Silver Greatsword"
ID=Imbued_Silver_Greatsword_01
Description = "A greatsword forged of silver and magic."
Quality = "Rare"
[Vendor Attributes]
SellPrice = 20,40,0
[Logic]
Cursed = false
NoDrop = false
Conjured = false
[Battle Attributes]
Damage=20-50
Type=Sword
Style=TwoHanded
[Attributes]
Strength = 5
Agility = 2
Stamina = 4
Intellect = 0
Wisdom = 0
Spirit = 0
[Attribute Buffs]
Strength = 5
Agility = 2
Stamina = 4
Intellect = 0
Wisdom = 0
Spirit = 0
[Attribute Debuffs]
Strength = 0
Agility = 0
Stamina = 0
Intellect = 0
Wisdom = 0
Spirit = 0
[Creature Potency]
Beast = 0
Undead = 50
Demon = 50
Dragon = 0
Human = 0
Magical = 50
Summoned = 50
[Creature Impotency]
Beast = 0
Undead = 0
Demon = 0
Dragon = 0
Human = 0
Magical = 0
Summoned = 0
[Resistances]
Fire = 0
Frost = 0
Energy = 0
Nature = 0
Arcane = 0
Holy = 0
Shadow = 0
[Aversions]
Fire = 0
Frost = 0
Energy = 0
Nature = 0
Arcane = 0
Holy = 0
Shadow = 0

Is there any way at all that my code could be improved?

\$\endgroup\$
  • \$\begingroup\$ Are you sure ini is the right type for your game? For example, try to define a list of enemies each with different attributes. A lot easier to do in a file that supports hierarchies. I also find the speed argument bogus. This initialization file is only parsed once (when the game is loading) reading and parsing a file of a few kb will not add any noticable effect to a loading screen, even if the format is slightly harder to parse. (Which it isn't really, since you can parse both ini and javascript without having to look back). \$\endgroup\$ – Roy T. Jan 10 '17 at 9:36
  • 3
    \$\begingroup\$ Definitely a reinvention of the wheel. There are lots of INI-parsing libraries for .NET, like this one. Avoid ones that call the legacy Win32 functions GetPrivateProfileString and WritePrivateProfileString, though. These are deprecated for good reason. But that doesn't mean INI files are. I believe they have lots of advantages for simple programs, as long as you are aware of the disadvantages. \$\endgroup\$ – Cody Gray Jan 10 '17 at 10:18
  • 1
    \$\begingroup\$ @CodyGray but the OP is not using these APIs. \$\endgroup\$ – t3chb0t Jan 10 '17 at 15:23
  • 1
    \$\begingroup\$ I agree, if you're serious about performance. In some cases, I am very serious about performance, and have re-invented many wheels that drastically outperform the original. So I'm not one of those people parroting well-meaning but useless advice. The important thing is not to waste time and energy optimizing code whose performance is irrelevant, which would be anything involving file I/O. Unless the code is pathologically written, it is not the bottleneck. Trust me. I almost rewrote my own INI parser for C++, but used a profiler to convince myself to spend the time on something else. \$\endgroup\$ – Cody Gray Jan 11 '17 at 7:53
  • 1
    \$\begingroup\$ You also have to be careful with "common sense" assumptions, and even more dangerous ones, like that shorter code (fewer lines) will be faster. These are seductive because they seem like they should be true, but dangerous because they rarely are. In fact, I disagree with the statement that "If you're serious about performance, nothing is better than sitting down and writing everything from scratch." In more cases than not, you end up with something that performs worse, because most people do not understand what makes fast code fast. Maybe you're the exception, but it doesn't look like it. \$\endgroup\$ – Cody Gray Jan 11 '17 at 7:55
8
\$\begingroup\$

The HasKey(string,string) can be simplified like so

public bool HasKey(string header, string key)
{
    return HasHeader(header) && this.Data[header].ContainsKey(key);
}

because if you have an if construct like

if (condition)
{
    return true;
}
return false;

you can simply return the condition.


The GetValue(string,string) is calling HasHeader() although this is done by the hasKey() method as well. So we can clean it like so

    public string GetValue(string header, string key)
    {
        if (HasKey(header, key))
        {
            return this.Data[header][key];
        }
        throw new KeyNotFoundException();
    }  

throw new Exception("No Header is Mounted!");  

This screems for a custom exception, maybe named NoHeaderException. You should always throw a specific exception, this has the advantage that if you enclose a call to a method in a try..catch method you can catch more distinctly and you don't have the need for catching a System.Exception.

See also why-are-we-not-to-throw-these-exceptions


\$\endgroup\$
6
\$\begingroup\$
public Dictionary<string, Dictionary<string, string>> Data { get; set; }

This is a very long definition. Create a new type for it:

class DataDictionary : Dictionary<string, Dictionary<string, string>> { }

then you can shorten it to

public DataDictionary Data { get; set; }

and it will simplify other parts of your code:

Data = new DataDictionary();

public InitializationFile(string path, bool loadImmediate = false)
{
    this.Data = new Dictionary<string, Dictionary<string, string>>();
    this.Source = path;
    if (loadImmediate)
    {
        Import(path);
    }
}

public InitializationFile()
{
    this.Data = new Dictionary<string, Dictionary<string, string>>();
}

Chain the constructors and remove the loadImmediate.

public InitializationFile(string path, bool loadImmediate = false) : this()
{
    this.Source = path;
    if (loadImmediate)
    {
        Import(path);
    }
}

public InitializationFile()
{
    this.Data = new Dictionary<string, Dictionary<string, string>>();
}

The constructor shouln't do any work but initilize only properties/fields. If you want to read the file immediately then it's better to create a factory method like From that creates an instance and calls the Load method.


 foreach (KeyValuePair<string, Dictionary<string, string>> header in Data)

Don't you like var?

foreach (var header in Data)

public int KeyValueCount
{
    get
    {
        int count = 0;
        foreach (KeyValuePair<string, Dictionary<string, string>> header in Data)
        {
            foreach (KeyValuePair<string, string> keyValuePairs in header.Value)
            {
                count++;
            }
        }
        return count;
    }
}

You can turn this into:

return Data.Sum(x => x.Value.Count);

This reduces it to O(n) from the previous O(n * m).


return this.MountedHeader == header;

this is redundant everywhere and you are not using it consistently. For example only for properties and not for methods. But even for properties not always like here:

foreach (KeyValuePair<string, Dictionary<string, string>> header in Data)

It'd be better to not use it at all.


public string[] GetKeys()
{
    if (this.HasHeaderMounted())
    {
        return GetKeys(this.MountedHeader);
    }
    throw new Exception("No Header is Mounted!");
}

In such cases the InvalidOperationException should be thrown. The simple Exception should never be used. If you don't find any appropriate exception then create your own.

\$\endgroup\$
  • 3
    \$\begingroup\$ If I found the type name too long I would use a using alias directive instead of subclassing a standard collection: using DataDictionary = System.Collections.Generic.Dictionary<string, System.Collections.Generic.Dictionary<string, string>>; \$\endgroup\$ – Johnbot Jan 10 '17 at 12:47
  • 1
    \$\begingroup\$ @Johnbot I find subclassing better in situations where you are going to use a type in multiple files. You don't have to define the alias everywhere. But sure, aliases is a good idea indeed. In fact I use it quite often myself but usually only within a single file. \$\endgroup\$ – t3chb0t Jan 10 '17 at 12:52
5
\$\begingroup\$

It's great that you are doing everything from scratch - your engine, your file parser, etc. but you might want to consider using XML or JSON

  • Both formats support hierarchy.

  • They offer serialization/deserialization.

  • They are easy to read, especially JSON.

But if you don't like that idea and want to stick to your way of doing things, there are few things you can change in your current code :

bool ContainsFast(string data, char delimiter) is also know as IEnumerable<>.Any() :

Your code :

private static bool ContainsFast(string data, char delimiter)
{
    for (int i = 0; i < data.Length; i++)
    {
        if (data[i] == delimiter)
        {
            return true;
        }
    }
    return false;
}

Can be transformed to a single line :

private static bool ContainsFast(string data, char delimiter)
{
    return data.Any(t => t == delimiter);
}

The performance loss of using LINQ here would be negligible.

int KeyValueCount I will try to do some calculations. You currently have O(n*m) operation here, but we can improve that and at the same time shorten your code with some LINQ.

Here are the 2 LINQ options :

  1. ?Flatten the collection with .SelectMany() and obtain the length of that collection.

  2. Sum the length of all the value dictionaries.

Your code :

int count = 0;
foreach (KeyValuePair<string, Dictionary<string, string>> header in Data)
{
    foreach (KeyValuePair<string, string> keyValuePairs in header.Value)
    {
        count++;
    }
}
return count;

Option 1:

return Data.SelectMany(header => header.Value).Count();

Note that .Count() and .SelectMany() are both O(n) operations.

Option 2:

return Data.Sum(header => header.Value.Count);

Note that .Count here is O(1) operation, and the whole return statement is turned into O(n) operation.

And now your property looks nice and simple :

public int KeyValueCount => Data.Sum(header => header.Value.Count);

Notice the expression body there ? You can apply the same to all of your get only properties e.g :

public int HeaderCount
{
    get
    {
        return Data.Count;
    }
}

Can become:

public int HeaderCount => Data.Count;

Issue

Lastly this property functions in a weird way :

private string _fileName;
public string FileName
{
    get { return _fileName; }
}

It's weird because you are actually setting _fileName somewhere else outside the actual property which is supposed to "encapsulate" behaviour, the only place where you should access the backing field instead of the property itself is inside the property to avoid endless recursion.

You have that line :

_fileName = Path.GetFileName(path);

If the reason why you needed to declare explicit backing field is to ensure you property will be set only inside this class you should instead apply the respective access modifier to your setter :

public string FileName { get; private set; }

It's shorter, it's issue-free and you will never access the backing field yourself.

\$\endgroup\$
1
\$\begingroup\$

Could save a bit of code with

public Dictionary<string, Dictionary<string, string>> Data { get; } = new Dictionary<string, Dictionary<string, string>>();

You are never actually assigning Data

\$\endgroup\$
  • \$\begingroup\$ I think you are mistaken; it's in the constructor. \$\endgroup\$ – Krythic Jan 10 '17 at 19:23
  • \$\begingroup\$ @Krythic I think you are mistaken with the meaning of "save a bit of code". \$\endgroup\$ – paparazzo Jan 10 '17 at 19:25
  • \$\begingroup\$ Define: "You are never actually assigning Data" \$\endgroup\$ – Krythic Jan 10 '17 at 19:35
  • \$\begingroup\$ @Paparazzi you can make Data readonly now. \$\endgroup\$ – Denis Jan 10 '17 at 19:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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