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?

share|improve this question
    
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). – Roy T. 10 hours ago
3  
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. – Cody Gray 9 hours ago
1  
@CodyGray but the OP is not using these APIs. – t3chb0t 4 hours ago
    
Yes, I'm aware of that. My comment recommended looking for INI libraries that have already been written, but since many of them use these deprecated Win32 APIs, I thought it best to explicitly caution against using them. The one I linked to does not use them. @t3c – Cody Gray 4 hours ago
    
@CodyGray "Don't reinvent the wheel" is one of those sayings that gets regurgitated way too much by novice programmers that it has somehow become a set-in-stone rule. The problem with most code is that it is too general purposed, thus you loose performance due to the verbosity. I have another post on this site goo.gl/WoPwc1 which is actually an inlined Dictionary with all the unnecessary bits trimmed out. It performs twice as fast as a Dictionary, no joke. If you're serious about performance, nothing is better than sitting down and writing everything from scratch. – Krythic 1 hour ago

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


share|improve this answer
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.

share|improve this answer
3  
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>>; – Johnbot 7 hours ago
1  
@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. – t3chb0t 7 hours ago

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.

share|improve this answer

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

share|improve this answer
    
I think you are mistaken; it's in the constructor. – Krythic 47 mins ago
    
@Krythic I think you are mistaken with the meaning of "save a bit of code". – Paparazzi 45 mins ago
    
Define: "You are never actually assigning Data" – Krythic 35 mins ago
    
@Paparazzi you can make Data readonly now. – denis 22 mins ago

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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