HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMajor

Constructing a query string using StringBuilder

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
queryconstructingusingstringbuilderstring

Problem

I speedily wrote a bunch of crappy checks, to check whether strings are empty, in order to build a query string.

StringBuilder SB = new StringBuilder();

SB.AppendFormat("{0}guide/search.aspx?", root);


root is a const, containing the website address.

It's like this because we have test servers.

if (!String.IsNullOrEmpty(item)) {
    SB.AppendFormat("&i={0}", Server.UrlEncode(item));
}
if (!String.IsNullOrEmpty(found)) {
      SB.AppendFormat("&f={0}", Server.UrlEncode(found));
}
if (!String.IsNullOrEmpty(what)) {
      SB.AppendFormat("&wt={0}", Server.UrlEncode(what));
}
if (!String.IsNullOrEmpty(where)) {
      SB.AppendFormat("&wr={0}", Server.UrlEncode(where));
}


I tried replacing these with ternaries, but I couldn't get my head around how to do that with the sb.AppendFormat in place.

  • Is there a better way to check empty strings than -String.IsNullOrEmpty?



  • Is my method of naming variables correct with C# standards?



  • Is Server.UrlEncode the best way to format strings into query strings?

Solution

Although there's nothing wrong with your code, it is generally easier to fall back on framework classes for this kind of thing. Namely UriBuilder and HttpValueCollection.

public static Uri BuildUri(string root, NameValueCollection query)
{
    // omitted argument checking

    // sneaky way to get a HttpValueCollection (which is internal)
    var collection = HttpUtility.ParseQueryString(string.Empty);

    foreach (var key in query.Cast().Where(key => !string.IsNullOrEmpty(query[key])))
    {
       collection[key] = query[key];
    }

    var builder = new UriBuilder(root) { Query = collection.ToString() };
    return builder.Uri;
}


Then you can simply use with a name value collection:

var values = new NameValueCollection 
{ 
    {"i", "item" }, 
    { "f", "found" },
    { "wt", "what%" },
    { "wr", "where" }
}; 
BuildUri("http://example.com/search.aspx", values);
// http://example.com/search.aspx?i=item&f=found&wt=what%25&wr=where


The HttpValueCollection handles all of the encoding for you so you don't need to worry about doing it the right way. You just create the name value collection and any null/empty values will just be skipped in the BuildUri method.

Update

As svick has pointed out in the comments it might not be ideal to use an internal class in this way. I think the risk of the implementation changing is quite low but it is a risk and should be thought about.

Code Snippets

public static Uri BuildUri(string root, NameValueCollection query)
{
    // omitted argument checking

    // sneaky way to get a HttpValueCollection (which is internal)
    var collection = HttpUtility.ParseQueryString(string.Empty);

    foreach (var key in query.Cast<string>().Where(key => !string.IsNullOrEmpty(query[key])))
    {
       collection[key] = query[key];
    }

    var builder = new UriBuilder(root) { Query = collection.ToString() };
    return builder.Uri;
}
var values = new NameValueCollection 
{ 
    {"i", "item" }, 
    { "f", "found" },
    { "wt", "what%" },
    { "wr", "where" }
}; 
BuildUri("http://example.com/search.aspx", values);
// http://example.com/search.aspx?i=item&f=found&wt=what%25&wr=where

Context

StackExchange Code Review Q#91783, answer score: 22

Revisions (0)

No revisions yet.