Попробуйте поймать: это приемлемая практика? - PullRequest
12 голосов
/ 18 сентября 2011

Мы получили код Java от поставщика программного обеспечения.Он содержит много блоков try-catch, но в части catch ничего нет.Они повсюду.Пример:

        try {
            spaceBlock.enable(LindsayModel);
        } catch (Exception e) {
        }

Мои вопросы: является ли вышеуказанная приемлемая практика?Если да, то когда?Или я должен просто удалить все эти «фальшивые» заявления try и catch?

Для меня это выглядит ужасной практикой, но я не достаточно опытен в Java, чтобы сказать наверняка.Зачем ловить ошибки, если вы не собираетесь ничего с ними делать?Мне кажется, вы сделали бы это только в том случае, если бы были уверены, что исключение не будет иметь абсолютно никаких последствий, и вам все равно, если оно произойдет.Тем не менее, это не совсем так в нашем конкретном приложении.

РЕДАКТИРОВАТЬ Чтобы дать некоторый контекст: мы купили продукт с поддержкой Java-сценариев у поставщика.Наряду с продуктом они предоставили большой сценарий проверки концепции, адаптированный к нашим потребностям.Этот скрипт вышел «бесплатно» (хотя мы бы не купили продукт, если бы он не шел вместе со скриптом), и он «работает».Но сценарий - это настоящая боль, из-за многих вещей, которые даже я, как новичок в Java, признаю ужасной практикой, один из которых - это фиктивный бизнес, пытающийся поймать.

Ответы [ 7 ]

19 голосов
/ 18 сентября 2011

Это действительно ужасная практика.Особенно ловля Exception, а не что-то конкретное, выделяет ужасный запах - даже NullPointerException будет проглочен.Даже если есть уверенность в том, что конкретное выброшенное исключение не имеет реального значения, всегда следует регистрировать его как минимум:

try {
    // code
}
catch (MyInconsequentialException mie) {
   // tune level for this logger in logging config file if this is too spammy
   MY_LOGGER.warning("Caught an inconsequential exception.", mie);
}

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

Одно важное различие заключается в том, используются ли try / catch для проглатывания проверенных исключений.Если это так, то это, вероятно, указывает на крайнюю апатию со стороны программиста - кто-то просто хотел, чтобы его / ее код компилировался.По крайней мере, код должен быть изменен:

try {
   // code
}
catch (SpecificCheckedException sce) {
   // make sure there is exception logging done farther up
   throw new RuntimeException(sce);
}

Это перезапустит исключение, заключенное в непроверенный RuntimeException, эффективно позволяя скомпилировать код.Тем не менее, даже это может считаться бандой. Лучшая практика для проверенных исключений - обрабатывать их на индивидуальной основе, либо в текущем методе, либо дальше, добавляя throws SpecificCheckedException к сигнатуре метода.

As @ Tom Hawtin упомянуто, new Error(sce) может использоваться вместо new RuntimeException(sce), чтобы обойти любые дополнительные Exception ловушки дальше, что имеет смысл для чего-то, что не должно быть брошено.

Если try / catch не используется для проглатывания проверенных исключений, это одинаково опасно и должно быть просто удалено.

8 голосов
/ 18 сентября 2011

Ужасно, действительно.Глотать подобное исключение может быть опасно.Как вы узнаете, что случилось что-то плохое?

Я бы почувствовал себя лучше, если бы продавец написал комментарии к документу и признал это («Мы знаем, что мы делаем»).Я бы почувствовал себя еще лучше, если бы в коде была очевидная стратегия борьбы с последствиями.Оберните его в RuntimeException и перебросьте;установите возвращаемое значение на соответствующее значение.Что-нибудь!

"Повсюду"?Есть ли несколько блоков try / catch, засоряющих код?Лично мне не нравится эта идиома.Я предпочитаю по одному на метод.

Может быть, вам стоит найти нового продавца или написать свой.

2 голосов
/ 18 сентября 2011
    try {
        meshContinuum.enable(MeshingModel);
    } catch (Exception e) {
    }

Это похоже на незаконченный код.Если метод enable генерирует исключение, он будет пойман и поглощен кодом.Если этого не произойдет, то нет смысла пытаться отловить непериодическое исключение.

Проверьте, чтобы увидеть методы и где их подписи не сопровождаются throws exceptionName, затем удалите пустые операторы try-catch из мест, где они называются.

Теоретически можно поставить try-поймать вокруг любого заявления.Компилятор не будет жаловаться на это.Однако это не имеет смысла, поскольку таким образом можно скрыть реальные исключения.

Это можно видеть как признак плохого качества кода.Вы, вероятно, должны быть готовы столкнуться с проблемами другого типа.

1 голос
/ 18 сентября 2011

На что указывает ваш контракт с поставщиком? Если то, что они написали, плохая практика и все остальное, соответствует спецификации, то они будут взимать плату за переписывание.

Лучше указать набор тестов, которые войдут во многие или все эти блоки try-catch и, следовательно, не пройдут. Если тесты не пройдены, у вас есть лучший аргумент, чтобы заставить их исправить свой ужасный код.

1 голос
/ 18 сентября 2011

К сожалению, вы не можете просто удалить его, потому что блок try генерирует проверенное исключение, тогда оно должно быть объявлено в предложении throws метода.При этом вызывающие метод должны будут catch его (но не в том случае, если вызывающие также имеют эту мерзость catch (Exception e) {}).

В качестве альтернативы рассмотрите возможность замены его на

try {
    meshContinuum.enable(MeshingModel);
} catch (Exception e) {
    throw (e instanceof RuntimeException) ? (RuntimeException) e : new RuntimeException(e);
}

Поскольку RuntimeException (и классы, его расширяющие) являются непроверенными исключениями, их не нужно объявлять вбросает оговорку.

1 голос
/ 18 сентября 2011

Это не самое лучшее:

  • Оно скрывает свидетельство исключения, поэтому отладка сложнее

  • Это может привести к автоматическому отказу функций

  • Это говорит о том, что автор, возможно, действительно хотел обработать исключение, но никогда не обходил его

Таким образом, могут быть случаи, когда этовсе в порядке, например, исключение, которое на самом деле не имеет никакого значения (на ум приходит Python mkdirs, который выдает исключение, если каталог уже существует), но обычно это не так здорово.

0 голосов
/ 18 сентября 2011

Ужасная идея на первый взгляд, полностью зависит от того, что вы на самом деле называете. Обычно это делается из-за лени или плохих привычек.

...