tags:

views:

215

answers:

10

I have following data:

Dictionary<string,string> dctParameters = new Dictionary(){
 {"a",var1},{"b",var2},{"c",var3},....
}

I want to join the "dctParameters" into a querystring.

What's the fastest / best among the following ways? Can you think you of a better way to do this?

1st method:

StringBuilder data = new StringBuilder();
string result = dctParameters.Aggregate(data, (x, pair) => data.Append(pair.Key).Append("=").Append(pair.Value).Append("&")).ToString();

2nd method:

StringBuilder data = new StringBuilder();
foreach (var item in dctParameters)
{
   data.Append(string.Format("{0}={1}&",item.Key, item.Value));
}
string result = data.ToString();

3rd method:

StringBuilder data = new StringBuilder();
foreach (var item in dctParameters)
{
     data.Append(item.Key).Append("=").Append(item.Value).Append("&");
}
string result = data.ToString();
+6  A: 

To be honest, it looks to me like you are trying to prematurely optimize.

I don't believe there will be a significant difference between these, but you could benchmark them using timers. I'd suggest therefore picking the one which you believe will be most readable for other developers who may need to maintain the code.

Russ Cam
A: 

I would guess your third method has the least overhead, but it could be a close tie to your second. I don't know what Append() is doing under the covers or what String.Format is doing.

I like how your second method looks; it's the most readable to me.

Cory Larson
+4  A: 

Assuming that when you say "querystring", you're referring to parameters passed in a URL, then:

How about:

String.Join("&", dic.Select(kv => kv.Key + "=" + kv.Value).ToArray());
Eric Smith
Guffa
Eric Smith
Guffa
Quite right - for some reason I was thinking SQL, in which case this would have been horribly wrong anyway. Updated.
Eric Smith
A: 

Best is a pretty relative term in this case as they all do what you want. As far as speed is concerned, I don't think it is worth the effort to worry over a few milliseconds here and there. These collection are never going to be large enough for you to see any noticeable differences in performance.

Do what works first (without compromising good design) and then worry about optimizing when you need to.

Josh
A: 

3d because it avoid formatting (which is preprocessing of format expression) and in opposite to 1st doesn't uses redundant call of delegate

Dewfy
+1  A: 

All three methods are incorrect! You will get a spurious ampersand at the very end. It won't impact a browser but it's just bad form.

It's certainly not one that involves string.Format because that requires parsing through your format string. I personally would go with the third method and precalculate the total end string length to feed into the StringBuilder constructor. Even if it's roughly the correct size or just guessed then you keep from reallocating memory for the StringBuilder. Try something like 20 for each item in your dictionary, YMMV.

Others did and will talk of premature optimization but usage of string.Format is wholly unnecessary, not to mention incorrect in this case.

Colin Burnett
I have something similar to this http://geekswithblogs.net/thomasthedeuce/archive/2005/10/31/58741.aspxthat i use to deal with this type of situation.
YetAnotherDeveloper
All three methods gives an extra ampersand at the end of the string, not only the second.
Guffa
LOL, touche Guffa.
Colin Burnett
+5  A: 

How about an extension method as a variation of Eric's suggestion:

public static string ToQueryString(this Dictionary<string, string> dict)
{
    return '?' + string.Join("&", dict.Select(p => p.Key + '=' + p.Value).ToArray());
}

eg:

dctParameters.ToQueryString();

The performance cost of any of your implementations is really not worth losing sleep over considering that this kind of process is not likely to bog down a server. The issue is (as you say) better not faster and what you seem to be after is an elegant approach rather than the least CPU cycles.

grenade
+1  A: 

I fundamentally agree with Russ. Opt for the most readable and factor it in a method. You can always change your implementation if you run into performance issues.

As a side note, you would probably gain more performance by choosing another class to hold your data as enumerating is quite expensive. Think of ListDictionary or even KeyedCollection (which would require a custom Parameter class or structure).

Mac
A: 

If you really need the best performance (which seems to be premature optimization), I think that you should first calculate the size of the string, then create a string builder with that capacity. This will keep the string builder from reallocating, and it will also result in a string without any unused space in it:

int len = -1;
foreach (var item in dctParameters) {
   len += item.Key.Length + item.Value.Length + 2;
}
StringBuilder data = new StringBuilder(len);
foreach (var item in dctParameters) {
   if (data.Length > 0) data.Append('&');
   data.Append(item.Key).Append('=').Append(item.Value);
}
string result = data.ToString();
Guffa
Why the downvote? If you don't say what it is that you don't like it's rather pointless...
Guffa
A: 

Never mind performance, this is not going to be a bottleneck. Eric Smiths version seem to be the simplest and cleanest - go for that.

But remember to UrlEncode the values, otherwise you will get a problem if a value happen to contain & or whitespace or other illegal characters.

JacquesB