Is a JavaScript try-catch ignoring an expected occasional error bad practice?

Question

In JavaScript is it wrong to use a try-catch block and ignore the error rather than test many attributes in the block for null?

try{ 
   if(myInfo.person.name == newInfo.person.name
      && myInfo.person.address.street == newInfo.person.address.street
      && myInfo.person.address.zip == newInfo.person.address.zip) {
         this.setAddress(newInfo);
    } 
} catch(e) {} // ignore missing args

Answer 1

For the example given I would say it was bad practice. There are instances however where it may be more efficient to simply trap for an expected error. Validating the format of a string before casting it as a GUID would be a good example.

Answer 2

I would think that if you're going to catch the exception then do something with it. Otherwise, let it bubble up so a higher level can handle it in some way (even if it's just the browser reporting the error to you).

Answer 3

Yes. For one, an exception could be thrown for any number of reasons besides missing arguments. The catch-all would hide those cases which probably isn't desired.

Answer 4

If you expect a particular condition, your code will be easier to maintain if you explicitly test for it. I would write the above as something like

if(   myInfo && newInfo 
      && myInfo.person && newInfo.person
      && myInfo.person.address && newInfo.person.address
      && ( myInfo.person.name == newInfo.person.name
           && myInfo.person.address.street == newInfo.person.address.street
           && myInfo.person.address.zip == newInfo.person.address.zip
         )
) 
{
     this.setAddress(newInfo);
}

This makes the effect much clearer - for instance, suppose newInfo is all filled out, but parts of myInfo are missing? Perhaps you actually want setAddress() to be called in that case? If so, you'll need to change that logic!

Answer 5

On a related note, in IE, even though the specs say you can, you can not use a try/finally combination. In order for your "finally" to execute, you must have a catch block defined, even if it is empty.

//this will [NOT] do the reset in Internet Explorer
try{
  doErrorProneAction();
} finally {
  //clean up
  this.reset();
}

//this [WILL] do the reset in Internet Explorer
try{
  doErrorProneAction();
} catch(ex){
  //do nothing
} finally {
  //clean up
  this.reset();
}

Answer 6

You could always write a helper function to do the checking for you:

function pathEquals(obj1, obj2, path)
{
    var properties = path.split(".");
    for (var i = 0, l = properties.length; i < l; i++)
    {
        var property = properties[i];
        if (obj1 === null || typeof obj1[property] == "undefined" ||
            obj2 === null || typeof obj2[property] == "undefined")
        {
            return false;
        }

        obj1 = obj1[property];
        obj2 = obj2[property];
    }

    return (obj1 === obj2);
}

if (pathEquals(myInfo, newInfo, "person.name") &&
    pathEquals(myInfo, newInfo, "person.address.street") &&
    pathEquals(myInfo, newInfo, "person.address.zip"))
{
    this.setAddress(newInfo);
}