[Из песочницы] Возвращение GOTO
Сейчас все понимают, что использовать оператор 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 и содержать ваш код чистым, понятным и расширяемым.
