How to Avoid Throwing An Error When SQL Select Returns Nothing

Question

I read somewhere that one should never use error conditions as normal program flow. Makes excellent sense to me... But

C# application sitting on top of a MySQL db. I need to parse a string value into two parts, an ID, and a value. (The original data come from a Devonian database), then validate the value against a lookup table. So, a couple of original strings might look like this:

"6776 Purple People Eater"

"BIK Yellow Polka-Dot Bikini (currently in use)"

"DCP Deuce Coup"

So, my little utility parses each string into the ID and the description based on the index of the first space (fortunately, consistent). I then pass the ID to the lookup, get the new value and away we go.

Unfortunately, TPTB also decided that we no longer need no stinkin' Yellow Polka-Dot Bikinis (currently in use). So, BIK does not return a row. Here's a code snippet:

    foreach (string product in productTokens) {
      tempProduct = product.Trim();
      if (tempProduct.Length > 0) {
        if (tempProduct.Length < 10) {
          product_id = tempProduct;
        }
        else {
          int charPosition = tempProduct.IndexOf(" ");
          product_id = tempProduct.Substring(0, charPosition);
        }
        try {
          s_product = productAdapter.GetProductName(product_id).ToString();
        }
        catch (Exception e) {
          if (e.Message.ToString() == "Object reference not set to an instance of an object.") {
            s_product = "";
          }
          else {
            errLog.WriteLine("Invalid product ID " + e.Message.ToString());
            Console.WriteLine("Invalid product ID " + e.Message.ToString());
            throw;
          } //else
        } //catch
        if (s_product.Length > 0) {
          sTemp = sTemp + s_product + "; ";
        }
      } //if product.length > 0
    } //foreach product in productTokens

Really, really ugly! Particularly the part where I test for an invalid IDin the catch block. There simply must be a better way to handle this.

If anyone can help me out, I'd really appreciate it.

Thanks.

Answer 1

You shouldn't be calling ToString() at that point, first you should check if the value returned is null;

object productName = productAdapter.GetProductName(product_id);
if ( productName != null )
{
    s_product = productName.ToString();
}
else
{
    s_product = String.Empty;
}

Answer 2

Can't you simply check whether GetProductName returns null?

var productName = s_product = productAdapter.GetProductName(product_id);
if(productName == null) { ... do something }
else {
    string name = productName.ToString();
}

Answer 3

In addition to the recommendations of Rob and Marc, and based on your code above, I'd suggest another tweak; I'd be really surprised if productAdapter.GetProductName() doesn't already return a String, in which case calling ToString() on it is utterly redundant. If it does indeed already return a String, then your whole try/catch block becomes one line:

s_product = productAdapter.GetProductName(product_id) ?? string.Empty;

Also, I think it might be useful to mention something else that already returns a String - Exception.Message. So all of the different places where you're calling ToString() in your code are also utterly redundant.

Also, I would suggest using the instance method String.Split() rather than the combination of IndexOf and SubString:

product_id = tempProduct.Split(" ", 2)[1];

Finally, deciding what kind of Exception you've caught by examining the Message property should be a totally last-ditch scenario. Even if you really did need to catch NullReferenceException here, you should do so explicitly:

catch (NullReferenceException) {
    s_product = "";
}
catch (Exception e) {
    // Log your invalid ID error condition here
}

P.S.: I'm also not really sure what at all this question has to do with MySQL, since there's no evidence of any DB API in your code.