Сейчас все понимают, что использовать оператор GOTO это не просто плохая, а ужасная практика. Дебаты по поводу его использования закончились в 80-х годах XX века и его исключили из большинства современных языков программирования. Но, как и положено настоящему злу, он сумел замаскироваться и воскреснуть в XXI веке под видом исключений.

Исключения, с одной стороны, являются достаточно простой концепцией в современных языках программирования. С другой же стороны, их часто используют неправильно. Есть простое и хорошо известное правило – исключения только для обработки поломок. И именно слишком вольная интерпретация понятия «поломка» приводит ко всем проблемам использования GOTO.

Теоретический пример

Разница между поломками и негативными бизнес-сценариями хорошо видна на окне входа в систему с очень простым сценарием использования:

1. Пользователь вводит логин/пароль.
2. Пользователь нажимает кнопку «Войти в систему».
3. Клиентское приложение отправляет запрос на сервер.
4. Сервер успешно проверяет логин/пароль (под успехом считает наличие соответствующей пары).
5. Сервер отсылает клиенту информацию, что аутентификация прошла успешно и ссылку на страницу перехода.
6. Клиент осуществляет переход на указанную страницу.

И одно негативное расширение:

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 и содержать ваш код чистым, понятным и расширяемым.