dotNetChris @ Marisic.Net

November 11, 2008

Working with QueryStrings, a critique and different solution.

Filed under: Programming — Tags: , , — dotnetchris @ 5:20 pm

Today I was reading DotNetKicks like I normally do and came across this article in the up and coming section, Parsing query string in ASP.NET safely. I felt the author had quite a bit of conflicting code in his blog post.

I was not a fan of having get properties that had associated private member backing fields but the backing fields weren’t even reliably used. The other issue I had was the fact the int? return property would default to 0 instead of null which overrides the point of using nullables, the last issue I had was the GET properties would cause conversion to be done on the RequestString every time.

private string QueryText
{
    get { return (string) (ViewState["ViewStateQueryText"] ??
 (ViewState["ViewStateQueryText"] = string.Empty)); }
    set { ViewState["ViewStateQueryText"] = value; }
}

private int? QueryId
{
    get { return (int?) (ViewState["ViewStateQueryId"]); }
    set { ViewState["ViewStateQueryText"] = value; }
}

private Guid? QueryGuid
{
    get { return (Guid?) (ViewState["ViewStateQueryGuid"]); }
    set { ViewState["ViewStateQueryGuid"] = value; }
}

protected void Page_Load(object sender, EventArgs e)
{
    if (!Page.IsPostBack)
    {
        QueryText = ConverQuerystringToString("QueryText");

        QueryId = ConverQuerystringToInt("QueryId");

        QueryGuid = ConvertQueryStringToGuid("QueryGuid");

        bool hasValue = QueryGuid.HasValue;
        hasValue = QueryId.HasValue;
    }
}

/// <summary>
/// Converts the query string to GUID.
/// </summary>
/// <param name="key">The key.</param>
/// <returns></returns>
private Guid? ConvertQueryStringToGuid(string key)
{
    if (string.IsNullOrEmpty(Request.QueryString[key]))
        return null;

    try
    {
        return new Guid(Request.QueryString[key]);
    }
    catch
    {
        return null;
    }
}

/// <summary>
/// Convers the querystring to string.
/// </summary>
/// <param name="key">The key.</param>
/// <returns></returns>
private string ConverQuerystringToString(string key)
{
    return Request.QueryString[key] ?? string.Empty;
}

/// <summary>
/// Convers the querystring to int.
/// </summary>
/// <param name="key">The key.</param>
/// <returns></returns>
private int? ConverQuerystringToInt(string key)
{
    int tmpParse;
    return int.TryParse(Request.QueryString[key], out tmpParse) ?
tmpParse : (int?) null;
}

This is how I handle reading the query string in my applications. If you’re wondering what’s going on inside the QueryText property I am doing 1 line lazy loading on the ViewState. This takes advantage of the lesser known fact that the assignment operator does not return void but actually returns itself which you can combine with the null coalescing operator to create 1 line lazy loading.

Let me know what you think about my practices. I love nullables for always having .HasValue and .GetValueOrDefault() to handle how I want to deal with defaults. I hate having to pick arbitrary values for default checks like -1 or 0 for ints etc.

UPDATE: 11/12

So I’m going to include a version for us on 3.5 where these methods are in an extension method over the querystring NameValueCollection.

public static class QueryStringExtensions
{
    /// <summary>
    /// Converts the query string to GUID.
    /// </summary>
    /// <param name="queryString">The query string.</param>
    /// <param name="key">The key.</param>
    /// <returns></returns>
    public static Guid? GetGuid(this NameValueCollection queryString, string key)
    {
        if (string.IsNullOrEmpty(queryString[key]))
            return null;

        try
        {
            return new Guid(queryString[key]);
        }
        catch
        {
            return null;
        }
    }

    /// <summary>
    /// Convers the querystring to string.
    /// </summary>
    /// <param name="queryString">The query string.</param>
    /// <param name="key">The key.</param>
    /// <returns></returns>
    public static string GetString(this NameValueCollection queryString, string key)
    {
        return queryString[key] ?? string.Empty;
    }

    /// <summary>
    /// Convers the querystring to int.
    /// </summary>
    /// <param name="queryString">The query string.</param>
    /// <param name="key">The key.</param>
    /// <returns></returns>
    public static int? GetInt(this NameValueCollection queryString, string key)
    {
        int tmpParse;
        return int.TryParse(queryString[key], out tmpParse) ?
tmpParse : (int?) null;
    }
}

Usage example (using same .aspx page as above)

protected void Page_Load(object sender, EventArgs e)
{
    if (!Page.IsPostBack)
    {
        QueryText = Request.QueryString.GetString("QueryText");

        QueryId = Request.QueryString.GetInt("QueryId");

        QueryGuid = Request.QueryString.GetGuid("QueryGuid");

        bool hasValue = QueryGuid.HasValue;
        hasValue = QueryId.HasValue;
    }
}

BloggingContext.ApplicationInstance.CompleteRequest();

kick it on DotNetKicks.com

About these ads

3 Comments »

  1. Cool post, but I would do a couple of things differently?

    1.) I don’t like using ViewState, it is hard to scale an application when it is dependent on ViewState. Look into storing values in Session, that way if you decide to go to memcached, NCache, or any other distributed cache you can without any impact on your code.

    2.) I think you could reduce the conversion to one generic method called Convert

    3.) You are using .Net 3.5 right?! Why not write an extension method that hangs right off of the Request.QueryString object. Just extend and overload the index operator, or add the convert method to the querystring.

    Request.QueryString[key];
    Request.QueryString["key"].Convert();

    that would be pretty sweet and convenient.

    Comment by Khalid Abuhakmeh — November 12, 2008 @ 8:52 am

  2. Khalid,

    I wrote this to be a basic interpreter for the query string that it’d be easy for anyone to grab the code. Yes I could have used extension methods but that would’ve narrowed down the availability of the code but I think I’ll add an update to include it also today. The only issue is it will still require the same amount of methods since .Convert() by itself won’t work unless I do some sort of type analysis on the data which would be problematic if people put a string instead of a number in the address bar it would still end up raising an exception when it tries to assign a string to a int variable.

    If I was writing an application and I had the requirement of being able to deal with changing the backing stores of a page what I’d most likely do is write up a bunch of wrappers for ViewState/Session/Cache as Unity LifeTimeManagers then I can change my backing stores with just a few line changes.

    This sounds like a good post about Unity especially with trying to figure out the best way to wrap the ViewState to be a LifeTimeManager. I have a few ideas I’ll have to play around with.

    Comment by dotnetchris — November 12, 2008 @ 9:16 am

  3. Ok So your comment thing stripped my generic brackets. This seemed like a perfect solution for generics, so I took the time to write a post about it. Check it Out.

    http://monstersgotmy.net/post/QueryString-Candy-Could-Rot-Your-Teeth.aspx

    Comment by Khalid Abuhakmeh — November 12, 2008 @ 10:09 pm


RSS feed for comments on this post. TrackBack URI

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

The Shocking Blue Green Theme Blog at WordPress.com.

Follow

Get every new post delivered to your Inbox.

Join 242 other followers

%d bloggers like this: