Как улучшить код и избавиться от множества if

306
10 декабря 2016, 11:12

Вопросы по оформлению кода...

Есть такой метод:

/**
 * Activate module from ZIP archive
 * @param $archiveFilePath
 * @return bool
 */
public function activate($archiveFilePath)
{
    if( file_exists($archiveFilePath) ) {
        $moduleDirectory = $this->extractArchive($archiveFilePath);
        if( $moduleDirectory ) {
            $moduleConfig = $this->getConfig($moduleDirectory);
            if( $moduleConfig ) {
            } else {
                \Yii::$app->session->setFlash(self::ERRORS_KEY, self::ERROR_INSTALL_CONFIG_NOT_EXIST);
                return false;
            }
        } else {
            \Yii::$app->session->setFlash(self::ERRORS_KEY, self::ERROR_ARCHIVE_EXTRACT);
            return false;
        }
    } else {
        \Yii::$app->session->setFlash(self::ERRORS_KEY, self::ERROR_ARCHIVE_NOT_EXIST);
        return false;
    }
}

Видим множество if. Мне постоянно необходимо проверять значения, которые возвращают другие методы класса и выводить ошибки, если что-то не так.

Вопрос 1. Код получается ужасный (это я только начал, там много таких проверок должно еще быть). Как в таких случаях поступать правильно?

Вопрос 2. Например, у меня есть проверка if( $moduleDirectory ). В $moduleDirectory может быть string или false. Стоит ли так писать? Или лучше делать так: if( $moduleDirectory !== false )?

Answer 1

Сделать классы исключений:

class ArchiveNotFoundException extends Exception;
class ArchiveExtractException extends Exception;
class ConfigNotFoundException extends Exception;

Функция, бросающаяся исключениями:

public function activate($archiveFilePath) {
    if (!file_exists($archiveFilePath)) 
        throw new ArchiveNotFoundException("Архив не найден");
    $moduleDirectory = $this->extractArchive($archiveFilePath);
    if (!$moduleDirectory) 
        throw new ArchiveExtractException("Ошибка распаковки архива");
    $moduleConfig = $this->getConfig($moduleDirectory);
    if (!$moduleConfig) 
        throw new ConfigNotFoundException("Конфигурация не найдена");
    // штатное продолжение функции
}

Применение функции, бросающейся исключениями:

try {
    // начало
    activate($archiveFilePath);
    // штатное продолжение программы
} catch (ArchiveNotFoundException $e) {
    \Yii::$app->session->setFlash(self::ERRORS_KEY, self::ERROR_ARCHIVE_NOT_EXIST);
} catch (ArchiveExtractException $e) {
    \Yii::$app->session->setFlash(self::ERRORS_KEY, self::ERROR_ARCHIVE_EXTRACT);
} catch (ConfigNotFoundException $e) {
    \Yii::$app->session->setFlash(self::ERRORS_KEY, self::ERROR_INSTALL_CONFIG_NOT_EXIST);
}

Кроме того функция теперь не привязана к YII (по крайней мере в части ошибок), сама не занимается их обработкой. Таким образом появляется возможность повторного использования кода в самых разных ситуациях и полная свобода действий по обработке ошибок.

Answer 2

Вопрос 1. Код получается ужасный (это я только начал, там много таких проверок должно еще быть). Как в таких случаях поступать правильно?

Придерживаться правила: сначала обрабатывать простые случаи, а потом, ниже по коду - сложные.

Вопрос 2. Например, у меня есть проверка if( $moduleDirectory ). В $moduleDirectory может быть string или false. Стоит ли так писать? Или лучше делать так: if( $moduleDirectory !== false )?

Всегда, конечно, лучше чтобы переменная имела всего 1 тип данных и все. Но если уж так, то: если строка не может быть пустой(пустая строка преобразуется в false) - то так вполне нормально, если может - проверять уже на равенство/не равенство.

Я бы код переписал так:

public function activate($archiveFilePath) {
    if( !file_exists($archiveFilePath) ) {
        \Yii::$app->session->setFlash(self::ERRORS_KEY, self::ERROR_ARCHIVE_NOT_EXIST);
        return false;
    }
    $moduleDirectory = $this->extractArchive($archiveFilePath);
    if( !$moduleDirectory ) {
        \Yii::$app->session->setFlash(self::ERRORS_KEY, self::ERROR_ARCHIVE_EXTRACT);
        return false;
    }
    $moduleConfig = $this->getConfig($moduleDirectory);
    if( !$moduleConfig ) {
        \Yii::$app->session->setFlash(self::ERRORS_KEY, self::ERROR_INSTALL_CONFIG_NOT_EXIST);
        return false;
    }
    // тут основной код - тот что внутри if( $moduleConfig ) ...
}
READ ALSO
Получить значение из объекта объектов

Получить значение из объекта объектов

Запутался в этих объектах, имея следующее, как получить значение id?

321
Как правильно объединить производителей и категории

Как правильно объединить производителей и категории

ЗдравствуйтеУ меня такая структура бд

302
Статик или обьект- как правильнее?

Статик или обьект- как правильнее?

Вот задался таким вопросом, где и как лучше использовать статик методы?Читал про них, но так и не понял где лучше использовать их, в каких ситуациях

360
PHP для Android

PHP для Android

Всем привет! Я хотел попробовать собрать php56 под андроид

307