The most 'elegant' way to populate a Dictionary<String,String> from a delimited string

Question

I think those with even a slight grasp on basic string manipulation, loops and dictionaries can work out how to populate a Dictionary from a String such as this:

Black:#00000|Green:#008000| (where "Black" is the Key and "#000000" is the Value)

But what is the most 'elegant' way of doing it in your opinion? What is the most efficient/more concise coding I can use to achieve it? So far I have:

    public static Dictionary<String, String> ThemeColors
    {
        get
        {
            Dictionary<String, String> themeColors = new Dictionary<string, string>();
            foreach (String colorAndCode in GetSettingByName("ThemeColors").ToString().Split('|'))
            {
                themeColors.Add(colorAndCode.Split(':').First(), colorAndCode.Split(':').Last());
            }
            return themeColors;
        }
    }

GetSettingByName("ThemeColours") returns the string above (in Bold).

It's functional obviously, it all works, but I want to make sure I'm beginning to think beyond this now and working out the best way of doing things rather than just getting it working.

Can I use a yield on the Dictionary loop for example??

Solution 1

I actually like a slight variant of your approach best:

    public static Dictionary<String, String> ThemeColors
    {
        get
        {
            Dictionary<String, String> themeColors = new Dictionary<string, string>();
            foreach (String colorAndCode in GetSettingByName("ThemeColors").ToString().Split('|'))
            {
                var parts = colorAndCode.Split(':');
                themeColors.Add(parts[0], parts[1]);
            }
            return themeColors;
        }
    }

The only difference is that the second Split() is done only once, and direct indexing is rather than First() and Last().

Now ToDictionary() is great when it makes sense to include something in a wider query, and I certainly wouldn't consider it wrong, but it's not like your approach is particularly verbose or anything.

But I like that it's easy to change your approach to tolerate duplicates (use dict[parts[0]] = parts[1] and it'll over-write duplicates rather than throwing), but it's also easy to change to throw for Black:#00000:#010101 by testing the size of parts.

In the other direction, if you've a need to get as quick a parsing of a massive string as possible, then throw elegance out the window and replace it with something that scans throught the string tokenising by | rather than using Split().

Meanwhile, the above fits a nice middle-ground between concision and precision.

Solution 2

 public static Dictionary<String, String> ThemeColors
    {
        get
        {
           return GetSettingByName("ThemeColors").ToString().Split('|').ToDictionary(colorAndCode => colorAndCode.Split(':').First(), colorAndCode => colorAndCode.Split(':').Last());
        }
    }

As suggested in comments more elegant way

 public static Dictionary<String, String> ThemeColors2
        {
            get
            {
                return GetSettingByName("ThemeColors").ToString().Split('|').Select(x => x.Split(new[] { ':' }, 2)).ToDictionary(x => x[0], x => x[1]);
            }
        }

Solution 3

Here is an alternative that uses a foreach. The answers by @Haris Hasan and @vcsjones are elegant in their own right. The reason I show this is that I encapsulate this logic in a static StringHelper class and this method can then be used as an extension of string.

The code from either answer above could be incorporated into this method.

Calling example:

var str = "Black:#00000|Green:#008000|";
Dictionary<string, string> dict = str.NameValuePairsToDictionary(":", "|");


    /// <summary>
    /// convert a string that consists of Name/Value Pairs to a Dictionary.
    /// Name and Value are separated by a given name/value separator
    /// Pairs are separated by a given pair separator token.
    /// example:
    /// Black:#00000|Green:#008000
    /// </summary>
    /// <param name="nvParis">string of name value pairs</param>
    /// <param name="nvSeparator">string that separates the name and value in a name value pair</param>
    /// <param name="pairSeparator">string that separates the name/value pairs from each other</param>
    /// <returns>Dictionary of Name value pairs as string,string</returns>
public static class StringHelper
{
    public static Dictionary<string, string> NameValuePairsToDictionary(this string nvPairs, string nvSeparator, string pairSeparator)
    {
        Dictionary<string, string> dict = new Dictionary<string, string>();

        // default - "\n\r"
        // split name value pairs by separator
        string[] items = nvPairs.Split(pairSeparator.ToCharArray());

        // for each split item, split the name/value pair by 
        // pair separator to add a dictionary item
        foreach (string item in items)
        {
            string[] keyVal = item.Split(nvSeparator.ToCharArray());
            if (keyVal.Length > 1)
                dict.Add(keyVal[0], keyVal[1]);
        }

        return dict;
    }
}

Solution 4

object settings = "Black:#00000|Green:#008000|";

var themeColors = (from setting in ((string)settings).Split('|')
                   where setting != ""
                   select setting.Split(new[]{':'}, 2))
                  .ToDictionary(a => a[0], a => a[1]);

View more solutions

Comments

[Admin]

Ahh, I've never even noticed the ToDictionary extension to be honest. Thanks a LOT!

[Random832]

I'd have done .Select(x=>x.Split(new[]{':'},2)).ToDictionary(x=>x[0],x=>x[‌​1])

[Massimiliano Peluso]

I think this solution is less readable and much slower then the solution that uses the foreach.

[Admin]

Without being bias - I would have to agree mine is more readable. I think that's obvious, but in terms of speed and efficiency I think Haris' answer is definitely an improvement.

[vcsjones]

@MassimilianoPeluso Slower, it's impossible to tell without measuring. I don't see why it would be any slower than the current solution. We could get a slight "boost" by not splitting on : twice using Select, but this is how I'd have done it. For people familiar with LINQ this is very readable.

[Matten]

You should use Split(new[] {"|"}, StringSplitOptions.RemoveEmptyEntries) to avoid errors.

[Massimiliano Peluso]

LINQ is always slower than a foreach(Linq to object uses For behind the scene) Refactoring the code splitting only once it would be enought. I think sometime people over-use LINQ just because is cool without caring about performances

[Haris Hasan]

@Massimiliano Peluso you are right but in most of the cases this slowness it would cause won't really matter. This phenomena is called premature optimizations

[Massimiliano Peluso]

in this case I don't see a reason to use LINQ which is much less readable than the solution using the classic ForEach :LINQ should be used to improve the readability or when what we are doing would be to complex to achieve with classic for + it is not possible to debug the LINQ solution We can't talk about "premature optimizations" in this case