Monday, January 29, 2007

Querystring: Laziness or Design?

{

Mads Kristensen posted about retrieving items from the querystring by a series of checks against nullness (is that a word?) and type values. His starts with:

if(!Request.QueryString["foo"] == null){
// do interesting things
}

Continues with a little more elegance:

if(String.IsNullOrEmpty(Request.QueryString["foo"])){
// do interesting things
}

Finishes with a check on type integrity during the querystring check:

if(!String.IsNullOrEmpty(Request.QueryString["foo"]) &&
Request.QueryString["foo"].length == 5){
// do interesting things
}

I like seeing things like this because it's interesting to look at how someone does something I do all the time with a different twist. I'm a pretty lazy guy, and although I hope to attribute it to the perl affinity in me, I'm not sure if it's really a spade being a spade and I actually am lazy in a bad way.

For most of my projects (except when I'm at a client or somewhere that I need to copy follow a defined pattern) I usually write a little "helper" class with static methods for this sort of thing:

class Helper{
public static string GetQS(Page page, string key){
return (page.Request[key] ?? String.Empty); // ?? is so slick yo.

}
}

When I'm reading out querystring values, I sometimes read and assign simultaneously:

string foo;
if((foo = Helper.GetQS(this, "foo"))!= String.Empty){
// do interesting things with foo
}

Of course this is a variation on Mad's theme, but one critical difference - a lazy one - is that my gut reaction is to intentionally allow failure on type integrity outside the querystring check. That failure usually invokes a more generic handler related to the operation in question rather than having to decide what happens the moment I know I have a bad querystring value. For example:

string quantity;
if((quantity = Helper.GetQS(this, "quantity"))!=String.Empty){
this.UpdateQuantity(Convert.ToInt32(quantity));
}

My rational is that I can write my exception handler for the operation "UpdateQuantity" rather than the specific querystring item. UpdateQuantity may throw different types of exceptions and I can capture them all in one fell swoop instead of defensively trying to test everything up front.

try{
string quantity;
if((quantity = Helper.GetQS(this, "quantity"))!=String.Empty){
this.UpdateQuantity(Convert.ToInt32(quantity));
}

}
catch(FormatException ex){
// specific exceptions I can of more assistance
}
catch(Exception ex){
// log and inform *shrug*
}

Of course that's lazy. I'd like to think it's lazy in a perl way rather than lazy in a couch potato way. What say you?

}

2 comments:

Big Red said...

why don't you just do?

string foo = Request["foo"] ?? string.Empty;

David Seruyange said...

Good point, very valid. Depending on the situation it could totally work. But a few things to consider:

1. In a separate method, you've got more wiggle room for your querystring check. For example, if you needed to check for an additional key in the querystring collection for security, it keeps the code for retrieval nice and tight. Alternatively, you could throw an exception for "required" items - an overload like:
Helper.GetQS(this,"key",Arg.Required);

2. You can implement changes in querystring checks across the entire app in one place.

Depends on the complexity of the app - what you wrote would (in my book) be good laziness if things were really that simple.