I came across this little dandy whilst browsing a very popular asp.net community site (code like this makes me want to beat whoever wrote it to death with a wet noodle); the question was “how do I test a string to see if it’s numeric”…one of the answers given was the following:

  
public bool IsNumeric(string s)
{
   try
   {
      Int32.Parse(s);
   }
   catch
   {
      return false;
   }
      return true;
} 

What…no finally block?  Just kidding.  Ahh, the proliferation of idiocy to up and coming developers at its finest.

Sidenote:  To anyone who doesn’t understand why the above code is oh so very evil, try/catch blocks should never ever be used to dictate control of flow or govern what a method returns.  Also, never catch an exception unless you are going to do something with it (repeat after me, thou shalt not swallow exceptions).  Not to mention the inherent processing overhead that the framework has to go through to catch such a worthless exception in this case (imagine if this block was called hundreds or thousands of times per second…ouch).  Resource-wise, generating exceptions are way up there. 

I personally would use double.TryParse() (and downcast accordingly depending on the result) at the very least.  At the very most I’d break the string down to a char array, and walk the array calling one of the (very) useful static char.Is<whatever> methods…first non<whatever> value, break out of the loop and return false.  I’ve posted before about the speed at which the framework can process char data…it’s very fast and effecient. 

The problem with solving something quick and dirty is that even after the quick, it’s still dirty.

Share this post: | | | | |