1. Java / Говнокод #6007

    +75

    1. 01
    2. 02
    3. 03
    4. 04
    5. 05
    6. 06
    7. 07
    8. 08
    9. 09
    10. 10
    11. 11
    12. 12
    13. 13
    14. 14
    private void configComponents(/* params */) throws MyException {
    	String err_msg = null;
    	try {
    		// some code here...
    		return;
    	} catch (ComponentConfigurationException e) {
    		err_msg = e.getMessage();
    	} catch (MyException e) {
    		err_msg = e.getMessage();
    	} catch (Exception e) {
    		err_msg = setupProcessErrorMessage;
    	}
    	throw new MyException(err_msg);
    }

    А rethrowing у нас уже не в почете. Правильно контрагаить ретурном из трай-секции и выбросом исключения, если трай-секция не дожила до ретурна.

    Запостил: mdcool, 16 Марта 2011

    Комментарии (12) RSS

    • >MyException
      до чего опротивели эти му.... Когда эксепшн мой? наверное, когда мозги не найдены.
      Ответить
      • Да какая разница, какой там эксепшн? Я про логику обработки.

        Собстно, в оригинале - не MyException, это вполне себе реальный проект, не позволил себе копировать не глядя.
        Ответить
    • Что не понравилось?
      Ответить
      • "Мы вот конфигурировали, конфигурировали, да не выконфигурировали" ©
        Ответить
      • То, что throw не место после catch-ей. Тем более с такой идиотской передачей сообщения об исключении. Увы, я нарвался именно на случай, когда getMessage() ретурнит null, и поиск этой проблемы без стектрейса занял пару часиков, пока не обратил внимание на вышесказанное.
        Ответить
        • Вопрос: почему getMessage() ретурнит null, кто за это ответственен и какое это имеет отношение к "throw не место после catch-ей" ?
          Откуда вообще предположение, что "throw не место после catch-ей" особенно глядя на сигнатуру функции?
          Ответить
          • > почему getMessage() ретурнит null
            Потому что кто-то до меня написал не-maintainable код, пользуясь throw new Exception(); . Он уже не работает у нас. Это другая песня. Я пояснял, в каких условиях нарвался на этот участок кода.

            > Откуда вообще предположение
            Из общих практик программирования. Такой переброс собщения об ошибке куда-то и создание исключения где-то там - это говнокод. А венчать трай-секцию ретурном только потому, что в конце функции обязательно выбросится исключение - еще ужаснее. Для этого и понаизобретено try-catch-finally с собственным набором правил.

            На случай сомнений: в моем комменте ниже приведен лаконичный читабельный код, который, казалось бы, делает то же самое, но очень помогает в поиске источника исключения. Им и была заменена эта жуткая конструкция во всех встреченных в проекте местах.
            Ответить
    • и что тут плохого? как сделать лучше? свой вариант в студию!

      мне не понравилось только то, что теряется исходный эксепшн. при отладке бывает очень нужен.
      Ответить
      • > Бывает очень нужен.
        Ага, при отладке. А при поиске, какого же фига конфигурация не загружается на конечном сервере, - просто необходим.

        По требованию трудящихся - свой вариант (достаточный для конкретно нашей системы):
        private void configComponents(/* params */) throws MyException {
        try {
        // some code here...
        } catch (Exception e) {
        throw new MyException(setupProcessErrorMessage, e);
        }
        }

        Решение, конечно, "в лоб", но для этой системы этого достаточно, чтобы вовремя определять причину падения компонентов. И без лапшичного кода.
        Ответить
        • 1. "При отладке" - я имел ввиду "при разбирательствах почему не работает".
          2. Ваш вариант не тождественен предыдущему. Мне был интересен именно тождественный код.

          В целом считаю говнокодом только потерю исходного эксепшена.
          Ответить
          • Вы просили свой, я свой и опубликовал :) На 99% тождественным будет следующий:

            private void configComponents(/* params */) throws MyException {
            try {
            // some code here...
            } catch (ComponentConfigurationException e) {
            throw new MyException(e.getMessage());
            } catch (MyException e) {
            throw new MyException(e.getMessage());
            } catch (Exception e) {
            throw new MyException(setupProcessErrorMessage);
            }
            }

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

    Добавить комментарий