Small Steps to More Robust Code

Most of us have seen code that takes a value from a user-entered field and uses it internally, and hopefully, it does some checking on this data first.  For web apps, this is especially true of Querystring and Post data. By not doing some simple checks, we open up the application to a large number of security vulnerabilities, as well as just simple points of failure.

A certain number of security concerns arise out of malicious data, such as SQL injection or cross site scripting. I'm not going to go into those here... it's not because they aren't valid, it's just that each one could take volumes of text in and of itself.

What am I going to talk about are a few, very simple things that can be done to help stabilize code, and these are things you can start doing today if you aren't already. So without further adieu:

1) Use TryParse.
How many times have I seen code that assumes a value with be in the valid range? Something like:

int a = int.Parse(Request.QueryString["id"]);

Ouch!  This isn't very robust because we're banking on the fact that the item in the Request collection exists, and that the string it contains is parsable.  Using TryParse, you can much more safely handle these situations, avoiding any costly and ugly exception. The TryParse method is available on virtually all primitive types, so this works on integers, longs, doubles, floats, datetimes (especially helpful), etc. You can improve this will something like:

int a;
bool IsInteger = int.TryParse(Request.QueryString["id"], out a);

... 'a' is an output parameter that will contain the integer, if the parse is successful. (If not, a is zero and IsInteger is false.)  If the value is not an integer, we can either stop processing, reassign it some other value, etc.:

if (!IsInteger)
{
   
//prompt the user
}

2) Check for Nulls.
I think the most common exception text I see (web app or otherwise) are these trusty words: "Object reference not set to an instance of an object."  As you likely know, this is runtime's way of saying, "the object you're trying to use is null, stupid."

There are a number of good techniques to help avoid these. First, cast reference type objects using "as," and then check for a null value. For example, suppose we want to pull an object out of a cache:

MyType data = Cache["data"] as MyType;

if
(data == null)
{
   data = MyDataLayer.LoadFromDatabase(id);
   Cache.Insert(data ...);
}

When casting using the "as" operation, the object your assigning to will always be null if the cast fails. No exceptions are thrown. The above snippet is a very common pattern for data access when the objects are cached.  Although many people do cast this way (especially useful for boxed objects) I often find that there is no subsequent null check.  Although there may be an exception here and there, it's imperative to look for null values, otherwise you might as well do an explicit cast.

Another way to check for nulls is to simply not assume the object you're accessing contains a value... for example:

string firstName = Request.QueryString["name"];

If "name" doesn't exist in the Request collection, you get a null reference exception when trying to use firstName. What you should do is:

if (Request.QueryString["name"] != null)
{
     string firstName = Request.QueryString["name"].ToString();
}
else
{
     ...
}

Seem like a pain? Yeah, but again, it's much more expensive to fix later on. This also provides an avenue for handling situations where the name is required.  (The ToString() is a bit redundant, but I prefer it because it's more clear from a readability point of view.)  This pattern is particularly useful in handling XML data, where you want to verify a node exists before accessing it's data. 

A good (and standard) design technique is to make sure any methods that return collection-based objects return empty collections instead of null when there is no data to populate. For example, suppose you have a method that returns a Hashtable with customer data based on the zip code passed in. If someone passes in 99999, you should return an empty Hashtable, not null. For example:

public static Hashtable GetCustomerData(string zip)
{
   Hashtable customerData = new Hashtable();
   //open db connection, query, etc.
   while (dataReader.Read())
   {
      customerData.Add(
...);
   }
   return customerData;
}

The above code can be wrapped in a try/catch block quite easily (in which case, you can decide whether to throw the exception or simply return an empty Hashtable).  You could argue you should always check for nulls, but generally developers just check the number of items in a collection, and if the count is zero, the appropriate message can be displayed.  Obviously, if null had been returned, accessing the count property would've throw the null reference exception.

3) Use Short Circuiting (to your advantage).
C# uses short circuiting by default: this means that in a conditional (such as an if statement) execution of a branch will begin as soon as enough of the criteria in the conditional are satisfied such that the rest of the criteria no longer need to be evaluated.  The rest of the conditions, if any, are simply ignored.  (You can do this in VB.NET, too, but the syntax is fairly silly with its OrElse and AndAlso keywords.)  So, let's look at this example:

int
someNumber;
int.TryParse(Request.QueryString["id"], out someNumber);
if (someNumber <= 0 || !IsValidIndex(someNumber))
{
    ...
}

If the value is less than or equal to zero, the first condition is satisfied and the call to IsValidIndex never happens (it doesn't matter if it's true or false, right?).  (Now, in a more real-world sense, we would probably add that <=0 check in the IsValidIndex and return false right away to avoid the database hit, but this is just for example purposes.)  Obviously, switching the order of these statements would defeat the purpose.

This technique can be combined with null checks.  For example, if you didn't follow the guideline (or don't want to trust it) that methods returning collections should never return null, and want to handle empty collections, you can do something like this:

Hashtable h = GetCustomerData("90210-1111");

if (h == null || h.Count == 0)
{
    //no items
}

This example combines a little safety checking in case the hashtable returned happens to be null.  The advantage is that peeking at the count never happens if the hashtable is null (in reality, this is more prevalent in custom data structures).

So there they are: 3 quick ways to help improve the robustness of your code!

Comments are closed

My Apps

Dark Skies Astrophotography Journal Vol 1 Explore The Moon
Mars Explorer Moons of Jupiter Messier Object Explorer
Brew Finder Earthquake Explorer Venus Explorer  

My Worldmap

Month List