Сейчас все понимают, что использовать оператор GOTO это не просто плохая, а ужасная практика. Дебаты по поводу его использования закончились в 80-х годах XX века и его исключили из большинства современных языков программирования. Но, как и положено настоящему злу, он сумел замаскироваться и воскреснуть в XXI веке под видом исключений.
Исключения, с одной стороны, являются достаточно простой концепцией в современных языках программирования. С другой же стороны, их часто используют неправильно. Есть простое и хорошо известное правило – исключения только для обработки поломок. И именно слишком вольная интерпретация понятия «поломка» приводит ко всем проблемам использования GOTO.
Теоретический пример
Разница между поломками и негативными бизнес-сценариями хорошо видна на окне входа в систему с очень простым сценарием использования:
- Пользователь вводит логин/пароль.
- Пользователь нажимает кнопку «Войти в систему».
- Клиентское приложение отправляет запрос на сервер.
- Сервер успешно проверяет логин/пароль (под успехом считает наличие соответствующей пары).
- Сервер отсылает клиенту информацию, что аутентификация прошла успешно и ссылку на страницу перехода.
- Клиент осуществляет переход на указанную страницу.
И одно негативное расширение:
4.1. Сервер не нашел соответствующую пару логин/пароль и посылает клиенту уведомление об этом.
Считать, что сценарий 4.1 является «проблемой» и поэтому его надо реализовывать с помощью исключения – достаточно распространенная ошибка. На самом деле это не так. Несоответствие логина и пароля – это часть нашего стандартного взаимодействия с пользователем, предусмотренная бизнес-логикой сценария. Наши бизнес-заказчики ожидают такого развития событий. Следовательно – это не поломка и использовать здесь исключения нельзя.
Поломки, это: разрыв соединения между клиентом и севером, недоступность СУБД, неправильная схема в БД. И еще миллион причин, ломающих наши приложения и не имеющих никакого отношения к бизнес-логике пользователя.
В одном из проектов, в разработке которого я участвовал, была более сложная логика входа в систему. Введя 3 раза подряд неправильный пароль, пользователь временно блокировался на 15 минут. Попадая 3 раза подряд во временную блокировку, пользователь получал постоянную блокировку. Также были дополнительные правила в зависимости от типа пользователя. Реализация с помощью исключений привела к тому, что внесение новых правил было крайне затруднительно.
Рассмотреть это пример было бы интересно, но он слишком большой и не очень наглядный. Как запутанный код с бизнес-логикой на исключениях превращается в понятный и лаконичный, я покажу на другом примере.
Пример Загрузка свойств
Попробуйте посмотреть данный код и четко понять, что он делает. Процедура не большая с достаточно простой логикой. При хорошем стиле программирования понимание ее сути не должно превышать больше 2-3 минут (я не помню сколько времени ушло у меня на полное понимание этого кода, но точно больше 15 минут).
private WorkspaceProperties(){ Properties loadedProperties = readPropertiesFromFile(WORK_PROPERTIES_PATH, true); //These mappings will replace any mappings that this hashtable had for any of the //keys currently in the specified map. getProperties().putAll( loadedProperties ); //Это файл имеет право отсутствовать loadedProperties = readPropertiesFromFile(MY_WORK_PROPERTIES_PATH, false); if (loadedProperties != null){ getProperties().putAll( loadedProperties ); } System.out.println("Loaded properties:" + getProperties()); } /** * Возвращает свойства, загруженные из указанного файла. * @param filepath * @param throwIfNotFound - кинуть FileNotFoundException, если файл не найден * @return Загруженные свойства или null, если файл не найден и !throwIfNotFound * @throws FileNotFoundException throwIfNotFound и файла с таким именем не надено * @throws IOException ошибка загрузки найденного файла */ private Properties readPropertiesFromFile(String filepath, boolean throwIfNotExists){ Properties loadedProperties = new Properties(); System.out.println("Try loading workspace properties" + filepath); InputStream is = null; InputStreamReader isr = null; try{ int loadingTryLeft = 3; String relativePath = ""; while (loadingTryLeft > 0){ try{ File file = new File(relativePath + filepath); is = new FileInputStream(file); isr = new InputStreamReader( is, "UTF-8"); loadedProperties.load(isr); loadingTryLeft = 0; } catch( FileNotFoundException e) { loadingTryLeft -= 1; if (loadingTryLeft > 0) relativePath += "../"; else throw e; } finally { if (is != null) is.close(); if (isr != null) isr.close(); } } System.out.println("Found file " + filepath); } catch( FileNotFoundException e) { System.out.println("File not found " + filepath); if (throwIfNotExists) throw new RuntimeException("Can`t load workspace properties." + filepath + " not found", e ); }catch (IOException e){ throw new RuntimeException("Can`t read " + filepath, e); } return loadedProperties; }
Итак, раскроем тайну – что же здесь происходит. Осуществляется загрузка свойств из двух файлов – обязательного WORK_PROPERTIES и дополнительного MY_WORK_PROPERTIES, добавляя в общее хранилище свойств. При этом есть нюанс – нам точно не известно, где лежит конкретный файл свойств – он может лежать как в текущем каталоге, так и в каталогах-предках (до трех уровней вверх).
Здесь смущает, как минимум, две вещи: параметр throwIfNotExists и большой блок логики в catch FileNotFoundException. Все это непрозрачно намекает – исключения используются для реализации бизнес-логики (а как иначе объяснить, что в одном сценарии выброс исключения – это поломка, а в другом – нет?).
Делаем правильный контракт
Сначала разберемся с throwIfNotExists. При работе с исключениями очень важно понимать – где именно его нужно обработать с точки зрения сценариев использования. В данном случае очевидно, что сам метод readPropertiesFromFile не может принять решение – когда отсутствие файла «плохо», а когда – «хорошо». Такое решение принимается в точке его вызова. По комментариям видно, что мы решаем – должен существовать этот файл или нет. Но на самом деле нам интересен не сам файл, а настройки из него. К сожалению, это никак не следует из кода.
Исправим оба этим недостатка:
Properties loadedProperties = readPropertiesFromFile(WORK_PROPERTIES_PATH); if (loadedProperties.isEmpty()) { throw new RuntimeException("Can`t load workspace properties"); } loadedProperties = readPropertiesFromFile(MY_WORK_PROPERTIES_PATH); getProperties().putAll( loadedProperties );
Теперь четко показана семантика –
WORK_PROPERTIES обязательно должны быть заданы, а MY_WORK_PROPERTIES — нет. Также при рефакторинге я обратил внимание, что readPropertiesFromFile никогда не сможет вернуть null и воспользовался этим при чтении MY_WORK_PROPERTIES.
Проверяем не ломая
Предыдущий рефакторинг также затронул и реализацию, но не значительно. Я просто удалил блок обработки throwIfNotExists:
if (throwIfNotExists) throw new RuntimeException(…);
Рассмотрев реализацию более пристально, мы начинаем понимать логику автора кода по поиску файла. Сначала проверяется, что файл находится в текущем каталоге, если не нашли – проверяем на уровне выше и т.д. Т.е. становится понятно, что алгоритм предусматривает отсутствие файла. При этом проверка делается с помощью исключения. Т.е. нарушен принцип – исключение воспринимается не как «что-то поломалось», а как часть бизнес-логики.
Существует функция проверки доступности файла для чтения File.canRead(). Используя ее можно избавиться от бизнес-логики в блоке catch
try{ File file = new File(relativePath + filepath); is = new FileInputStream(file); isr = new InputStreamReader( is, "UTF-8"); loadedProperties.load(isr); loadingTryLeft = 0; } catch( FileNotFoundException e) { loadingTryLeft -= 1; if (loadingTryLeft > 0) relativePath += "../"; else throw e; } finally { if (is != null) is.close(); if (isr != null) isr.close(); } }
Изменив код, получаем следующее:
private Properties readPropertiesFromFile(String filepath) { Properties loadedProperties = new Properties(); System.out.println("Try loading workspace properties" + filepath); try { int loadingTryLeft = 3; String relativePath = ""; while (loadingTryLeft > 0) { File file = new File(relativePath + filepath); if (file.canRead()) { InputStream is = null; InputStreamReader isr = null; try { is = new FileInputStream(file); isr = new InputStreamReader(is, "UTF-8"); loadedProperties.load(isr); loadingTryLeft = 0; } finally { if (is != null) is.close(); if (isr != null) isr.close(); } } else { loadingTryLeft -= 1; if (loadingTryLeft > 0) { relativePath += "../"; } else { throw new FileNotFoundException(); } } } System.out.println("Found file " + filepath); } catch (FileNotFoundException e) { System.out.println("File not found " + filepath); } catch (IOException e) { throw new RuntimeException("Can`t read " + filepath, e); } return loadedProperties; }
Также я снизил уровень переменных (is, isr) до минимально допустимого.
Такой простой рефакторинг значительно повышает читаемость кода. Код напрямую отображает алгоритм (если файл существует, то читаем, а иначе – уменьшаем количество попыток и ищем в каталоге выше).
Выявляем GOTO
Рассмотрим детально происходящее в ситуации, если файл не был найден:
} else { loadingTryLeft -= 1; if (loadingTryLeft > 0) { relativePath += "../"; } else { throw new FileNotFoundException(); } }
Видно, что здесь исключение используется для того, чтобы прервать цикл выполнения и фактически выполняют функцию GOTO.
Для сомневающихся сделаем еще одно изменение. Вместо использования мелкого костыля в виде loadingTryLeft = 0 (костыль, потому что на самом деле успешная попытка неизменяет количество оставшихся попыток) явно укажем, что считывание файла приводит к выходу из функции (не забыв при этом написать сообщение):
try { is = new FileInputStream(file); isr = new InputStreamReader(is, "UTF-8"); loadedProperties.load(isr); System.out.println("Found file " + filepath); return loadedProperties; } finally {
Это позволяет нам заменить условие while (loadingTryLeft > 0) на while(true):
try { int loadingTryLeft = 3; String relativePath = ""; while (true) { File file = new File(relativePath + filepath); if (file.canRead()) { InputStream is = null; InputStreamReader isr = null; try { is = new FileInputStream(file); isr = new InputStreamReader(is, "UTF-8"); loadedProperties.load(isr); System.out.println("Found file " + filepath); return loadedProperties; } finally { if (is != null) is.close(); if (isr != null) isr.close(); } } else { loadingTryLeft -= 1; if (loadingTryLeft > 0) { relativePath += "../"; } else { throw new FileNotFoundException(); // GOTO: FFN } } } } catch (FileNotFoundException e) { // LABEL: FFN System.out.println("File not found " + filepath); } catch (IOException e) { throw new RuntimeException("Can`t read " + filepath, e); }
Чтобы избавиться от явного дурно пахнущего throw new FileNotFoundException, нужно вспомнить контракт функции. Функция в любом случае возвращает набор свойств, если не смогли считать файл – возвращаем его пустым. Поэтому нет никаких причин выбрасывать исключение и перехватывать его. Достаточно обычного условия while (loadingTryLeft > 0):
private Properties readPropertiesFromFile(String filepath) { Properties loadedProperties = new Properties(); System.out.println("Try loading workspace properties" + filepath); try { int loadingTryLeft = 3; String relativePath = ""; while (loadingTryLeft > 0) { File file = new File(relativePath + filepath); if (file.canRead()) { InputStream is = null; InputStreamReader isr = null; try { is = new FileInputStream(file); isr = new InputStreamReader(is, "UTF-8"); loadedProperties.load(isr); System.out.println("Found file " + filepath); return loadedProperties; } finally { if (is != null) is.close(); if (isr != null) isr.close(); } } else { loadingTryLeft -= 1; if (loadingTryLeft > 0) relativePath += "../"; } } System.out.println("file not found"); } catch (IOException e) { throw new RuntimeException("Can`t read " + filepath, e); } return loadedProperties; }
В принципе, с точки зрения правильной работы с исключениями здесь все. Остается сомнение в необходимости выбрасывать RuntimeException в случае проблем IOException, но оставим его как есть ради совместимости.
Наводим лоск
Остались немного мелочей, исправив которые мы сделаем код еще более гибким и понятным:
- Название метода readPropertiesFromFile раскрывает его реализацию (кстати, равно как и throws FileNotFoundException). Лучше назвать более нейтрально и лаконично – loadProperties(…)
- Метод одновременно и ищет, и считывает. Для меня это две разных обязанности, которые можно разделить в разных методах.
- Изначально код писался под Java 6, а сейчас используется на Java 7. Это позволяет использовать closable resources.
- По опыту знаю, что при выводе информации о найденном или не найденном файле лучше использовать полный путь к файлу, а не относительный.
if (loadingTryLeft > 0) relativePath += "../";— если внимательно посмотреть код, то видно – эта проверка лишняя, т.к. при исчерпании лимита поиска все равно новое значение использовано не будет. А если в коде что-то лишнее, это мусор, который следует убрать.
Окончательная версия исходного кода:
private WorkspaceProperties() { super(new Properties()); if (defaultInstance != null) throw new IllegalStateException(); Properties loadedProperties = readPropertiesFromFile(WORK_PROPERTIES_PATH); if (loadedProperties.isEmpty()) { throw new RuntimeException("Can`t load workspace properties"); } getProperties().putAll(loadedProperties); loadedProperties = readPropertiesFromFile(MY_WORK_PROPERTIES_PATH); getProperties().putAll(loadedProperties); System.out.println("Loaded properties:" + getProperties()); } private Properties readPropertiesFromFile(String filepath) { System.out.println("Try loading workspace properties" + filepath); try { int loadingTryLeft = 3; String relativePath = ""; while (loadingTryLeft > 0) { File file = new File(relativePath + filepath); if (file.canRead()) { return read(file); } else { relativePath += "../"; loadingTryLeft -= 1; } } System.out.println("file not found"); } catch (IOException e) { throw new RuntimeException("Can`t read " + filepath, e); } return new Properties(); } private Properties read(File file) throws IOException { try (InputStream is = new FileInputStream(file); InputStreamReader isr = new InputStreamReader(is, "UTF-8")) { Properties loadedProperties = new Properties(); loadedProperties.load(isr); System.out.println("Found file " + file.getAbsolutePath()); return loadedProperties; } }
Резюме
Разобранный пример наглядно показывает, к чему приводит небрежное обращение с исходным кодом. Вместо использования исключения для обработки поломки, было принято решение использовать его для реализации бизнес-логики. Это сразу привело к сложности его поддержки, что и нашло отражение в его дальнейшей доработке под новые требования и в итоге – к отходу от принципов структурного программирования. Использование простого правила – исключения только для поломок – поможет вам избежать возвращения в эру GOTO и содержать ваш код чистым, понятным и расширяемым.
ссылка на оригинал статьи https://habr.com/ru/post/484840/
Добавить комментарий