Query Strings Must Die

Alright, I know they can't die entirely, but we need to insulate ourselves from them as much as possible.  I'll go in to more detail in another post about strategies for getting rid of the pesky punctuation point entirely, but here I just want to show a micro usage of the query string and how to use it better.

So, I was on a project the other day and I saw some code that wasn't spectacular.  It looked something like this (not syntactically correct, I'm doing it from memory):

       protected void Page_Load(object sender, EventArgs e)
        {
                     if (Request["id] == null)                      {
                           .....
                     }
        }

... then another method ...

      protected void DisplaySomething()
      {
           string id = (int)Request["id];
            .......
      }

… then in another event hanlder ...

     protected void btn_click(object sender, EventArgs e)
     {
         .....
                   if (Request["id] == null)
                   {
                           .....
                   }
         .....
     }

Can anyone tell me why this is bad??

A few reasons:

1. You are defining an API for your page, it accepts "id" and behaves differently with different arguments sent in. You probably don't think about this as an "API", but you should. Not only can other pages (other than the one you probably intend) call your page using this API, but END USERS can interact with this API by changing the text in their browser address bar. I'll talk about strategies to mitigate this in a separate blog post (probably aso called 'Query Strings Must Die')
2. If "id" every changes to "ID" or changes to "Identifier" or something else, you've got to change it in a bunch of places.

One strategy for mitigating this by putting the parameter name into a configuration file.  This way you can just call it using something like this: Request[IDIDENTIFER]

Another strategy is to use your code behinds as a class (which they most certainly are). You can put a property on your code behind that looks something like this:

      public string WorkingID
      {
          get { return this.m_WorkingID; }
          set { this.m_WorkingID = value; }
      }

then in your page_load, set it once like this:

        protected void Page_Load(object sender, EventArgs e)
        {
                     this.WorkingID = Request["id];         
        }

then in subsequent methods, you use the property like:

     protected void btn_click(object sender, EventArgs e)
     {
         .....
                   if (this.WorkingID == null)
                   {                          
                          .....
                   }    
     }

Another way yet to handle this (depending on your needs) is to just have the property itself wrap the handling and retrieval of the Request parameter as in:

      public string WorkingID
      {
           get { return Request["ID"]; }
      }

This way, if it ever changes, you change it one place and your code behind functionality won't be impacted.  This is a standard defensive codeing technique for any API you are building applied to the dreaded Query string.

Also, in general, there shouldn't be much need to use Query strings, especially in an event based architecture like .NET framework.  If you don't know the proper pattern for not using query strings, please ask me or look for a follow up posting.