Является ли JavaScript try-catch игнорированием ожидаемой случайной ошибки плохой практикой? - PullRequest
6 голосов
/ 26 сентября 2008

В JavaScript неправильно использовать блок try-catch и игнорировать ошибку вместо проверки множества атрибутов в блоке на ноль?

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

Ответы [ 6 ]

4 голосов
/ 26 сентября 2008

Если вы ожидаете определенного условия, ваш код будет легче поддерживать, если вы явно протестируете его. Я бы написал выше как что-то вроде

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);
} 

Это делает эффект намного более ясным - например, предположим, что newInfo полностью заполнен, но части myInfo отсутствуют? Возможно, вы действительно хотите, чтобы setAddress () вызывался в этом случае? Если это так, вам нужно изменить эту логику!

2 голосов
/ 26 сентября 2008

Да. С одной стороны, исключение может быть выдано по любому количеству причин, кроме отсутствующих аргументов. Всеобъемлющее скрыло бы те случаи, которые, вероятно, нежелательны.

1 голос
/ 26 сентября 2008

В соответствующей заметке в IE, хотя спецификации говорят, что вы можете, вы не можете использовать комбинацию try / finally. Для того, чтобы ваш «finally» выполнялся, у вас должен быть определен блок catch, даже если он пустой.

//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();
}
1 голос
/ 26 сентября 2008

Я думаю, что если вы собираетесь поймать исключение, сделайте что-нибудь с ним. В противном случае, пусть он всплывет, чтобы более высокий уровень мог каким-то образом справиться с ним (даже если это просто браузер сообщает об ошибке).

0 голосов
/ 27 сентября 2008

Вы всегда можете написать вспомогательную функцию для проверки:

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);
}
0 голосов
/ 26 сентября 2008

В приведенном примере я бы сказал, что это плохая практика. Однако есть случаи, когда может быть эффективнее просто перехватить ожидаемую ошибку 1004 *. Проверка формата строки перед приведением к GUID будет хорошим примером.

...