Constructing a query string using StringBuilder

Question

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?

Answer 1

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<string>().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.

Answer 2

how about:

QueryBuilder.For("guide/search.aspx")
    .Query("i", item)
    .Query("f", found)
    .Query("wt", what)
    .Query("wr", where)
    .ToString();

public class QueryBuilder
{
    public static QueryBuilder For(string page)
    {
        return new QueryBuilder(page);
    }

    private readonly StringBuilder sb;

    private QueryBuilder(string page)
    {
        sb = new StringBuilder()
                 .AppendFormat(CultureInfo.InvariantCulture,
                               "{0}{1}?",
                               root,
                               page);
    }

    public QueryBuilder Query(string queryName, string queryValue)
    {
        // todo: throw if parameter sb or queryValue is null

        if(!string.IsNullOrEmpty(queryValue))
        {
            sb.AppendFormat(CultureInfo.InvarianteCulture,
                            "&{0}={1}",
                            queryName,      
                            Server.UrlEncode(what));
        }

        return this;
    }

    public override string ToString()
    {
        return sb.ToString();
    }
}

regarding your questions:

• string.IsNullOrEmpty is ok. Depending on the use case string.IsNullOrWhitespace (=> null or empty or only whitespaces like space, tab, newline) may be a good fit too.
• local variable names should be lower case StringBuilder sb instead of StringBuilder SB

Answer 3

What you have is a collection of key-value pairs of strings, and you're performing the same operation on all of them. A simple improvement is to throw them into a collection and iterate over it.

var parameters = new List<KeyValuePair<string, string>>
{
    new KeyValuePair<string, string>("i", item), // "item"
    new KeyValuePair<string, string>("f", found), // null or ""
    new KeyValuePair<string, string>("wt", what), // "what"
    new KeyValuePair<string, string>("wr", where) // "where"
};

foreach (string parameter in parameters)
{
    if (!String.IsNullOrEmpty(parameter)) {
        SB.AppendFormat("&{0}={1}", parameter.Key, Server.UrlEncode(parameter.Value));
    }
}
// http://example.com/guide/search.aspx?&i=item&wt=what&wh=where

There is a problem with your code and the code above, though: the query string is always going to start with an ampersand. A quick and dirty solution would be to build out the query string first, and then take a substring to truncate the first character before appending it to the StringBuilder.

Another way to solve this is using Linq to filter and format the data, and then bringing it all together with a String.Join call, in which we can specify the ampersand as a separator.

var parameters = new List<KeyValuePair<string, string>>
{
    new KeyValuePair<string, string>("i", item), // "item"
    new KeyValuePair<string, string>("f", found), // null or ""
    new KeyValuePair<string, string>("wt", what), // "what"
    new KeyValuePair<string, string>("wr", where) // "where"
};

var validParameters = parameters.Where(p => !String.IsNullOrEmpty(p.Value));
var formattedParameters = validParameters.Select(p => p.Key + "=" + Server.UrlEncode(p.Value));
SB.Append(String.Join("&", formattedParameters));
// http://example.com/guide/search.aspx?i=item&wt=what&wh=where