Удобство чтения кода с конструкцией break

stalxed

Новичок
Очень часто вижу, что люди совмещают return, break и throw образуя кашу.
Яркий пример, над которым задумался сам. Да и написал сам...
PHP:
public static function decode($json)
{
    $data = json_decode(self::clean($json), true);
    switch (json_last_error()) {
        case JSON_ERROR_NONE:
            return $data;
            break;
        case JSON_ERROR_DEPTH:
            throw new Exception\RuntimeException(
                'Unable to decode the json string. The cause: maximum stack depth exceeded.'
            );
            break;
        case JSON_ERROR_STATE_MISMATCH:
            throw new Exception\RuntimeException(
                'Unable to decode the json string. The cause: underflow or the modes mismatch.'
            );
            break;
        case JSON_ERROR_CTRL_CHAR:
            throw new Exception\RuntimeException(
                'Unable to decode the json string. The cause: unexpected control character found.'
            );
            break;
        case JSON_ERROR_SYNTAX:
            throw new Exception\RuntimeException(
                'Unable to decode the json string. The cause: Syntax error, malformed JSON.'
            );
			break;
        case JSON_ERROR_UTF8:
            throw new Exception\RuntimeException(
                'Unable to decode the json string. The cause: malformed UTF-8 characters.'
            );
            break;
        default:
            throw new Exception\RuntimeException(
                'Unable to decode the json string. The cause is unknown.'
            );
            break;
    }
}
Другой пример, уже просто видел:
PHP:
while (same_condition) {
    if (some_condition) {
        return false;
        break;
    }
}
Во всех этих случаях break не нужен.
Но в том же switch ведь необходимо показать, что дальнейшие действия выполняться не должны не при каких обстоятельствах, и это показывает конструкция break, но с точки зрения ЯП throw сам выбьет конструкцию switch нафиг, как и return.
В PSRах это не написано, а жаль...
Кто что думает, необходим ли break в этих случаях?
 

stalxed

Новичок
Ха, в PSR2:
PHP:
switch ($expr) {
    case 0:
        echo 'First case, with a break';
        break;
    case 1:
        echo 'Second case, which falls through';
        // no break
    case 2:
    case 3:
    case 4:
        echo 'Third case, return instead of break';
        return;
    default:
        echo 'Default case';
        break;
}
break стоит в конструкции default, хотя по сути он там никогда не понадобится.
 

флоппик

promotor fidei
Команда форума
Партнер клуба
Очень часто вижу, что люди совмещают return, break и throw образуя кашу.
Яркий пример, над которым задумался сам. Да и написал сам...
Кто что думает, необходим ли break в этих случаях?
Плохой пример. Эту кучу throw потом *********** ловить и искать по строкам.
В кейсе можно подготовить код ошибки и текст, но бросать надо в одном месте.
 
Последнее редактирование модератором:

stalxed

Новичок
Чем будет лучше, когда текст ошибки и "бросание" исключения находятся в разных местах?

Ловить проще простого:
PHP:
try {
    \Utility\Json::decode($string);
} catch (\Utility\Exception\RuntimeException $e) {
    // no action, for example
}
Можно конечно сделать наследование от RuntimeException JsonException и прочее, но смысла лично в моём проекте нет.
 

hell0w0rd

Продвинутый новичок
лучше сделай ассоциативный массив код => описание и функцию getJsonError, в которую этот код передаешь. В последствии если решишь сделать JsonParseException - все будет ок)
 

stalxed

Новичок
флоппик, hell0w0rd
Оба ваших предложения написаны в комментариях http://www.php.net/manual/en/function.json-last-error-msg.php как раз 2 комментария именно с этими способами.

Что плохого в любви к эксепшенам? меньше всяких ифов, условных выражений, проверок и прочего становится.
Плюём как можно больше исключений, записываем в лог(try{ /** весь код приложения */ } catch (Exception $e) { /** пишем в лог */ }), и по логу уже постепенно начинаем обрабатывать нужные исключения нужным способом, создаём нужную иерархию исключений. Т.е. требовать иерархию исключений начинает клиентский код, а не код пакетов.

Но вопрос не в этом, например этот switch по сути читается как книга:
switch
если нет ошибок возвращаем данные
если ошибка JSON_ERROR_DEPTH, то эксепшен с сообщением о проблемах с глубиной JSON
если ошибка JSON_ERROR_STATE_MISMATCH, то эксепшен с сообщением о некорректном JSON
etc

Ну а if-elseif-else будет выключить уродливее.

Но каждый случай(case) switch предполагает продолжение в нижеследующем случае(case), если нет конструкции break. Понятно, что эксепшен "выбивает" конструкцию switch, но стоит ли всё таки оставлять break?

Готовится к тому, что будет в последствие не буду, когда клиентскому коду понадобится например различать ошибки json, вот тогда наследую соответствующие эксепшены RuntimeException этого неймспейса.
 

hell0w0rd

Продвинутый новичок
stalxed
кто против исключений? флоппик написал кидать в одном месте, а я посоветовал вместо кучи case брать значение из массива
 

Вурдалак

Продвинутый новичок
stalxed, философ, тебе говорят сделать так:
PHP:
public static function decode($json)
{
	$data = json_decode(self::clean($json), true);

	if (json_last_error() !== JSON_ERROR_NONE) {
		throw new Exception\RuntimeException(
		    self::getLastJsonErrorMessage(), json_last_error()
		);
	}

	return $data;
}

private static function getLastJsonErrorMessage()
{
	if (version_compare(PHP_VERSION, '5.5.0', '>=')) {
		return json_last_error_msg();
	} else {
		return [
            JSON_ERROR_DEPTH            => 'Maximum stack depth exceeded',
            JSON_ERROR_STATE_MISMATCH   => 'Underflow or the modes mismatch',
            JSON_ERROR_CTRL_CHAR        => 'Unexpected control character found',
            JSON_ERROR_SYNTAX           => 'Syntax error, malformed JSON',
            JSON_ERROR_UTF8             => 'Malformed UTF-8 characters, possibly incorrectly encoded'
		]
        [json_last_error()]
	}
}
 
Последнее редактирование:

stalxed

Новичок
Вурдалак, люблю совершенство, красивая конструкция, но я попросту перевёл например код с исходников https://github.com/php/php-src/blob/PHP-5.5.3/ext/json/json.c на php
PHP:
if (! function_exists('json_last_error_msg')) {
    function json_last_error_msg()
    {
        switch (json_last_error()) {
        	case JSON_ERROR_NONE:
        	    return 'No error';
        	case JSON_ERROR_DEPTH:
        	    return 'Maximum stack depth exceeded';
        	case JSON_ERROR_STATE_MISMATCH:
        	    return 'State mismatch (invalid or malformed JSON)';
        	case JSON_ERROR_CTRL_CHAR:
        	    return 'Control character error, possibly incorrectly encoded';
        	case JSON_ERROR_SYNTAX:
        	    return 'Syntax error';
        	case JSON_ERROR_UTF8:
        	    return 'Malformed UTF-8 characters, possibly incorrectly encoded';
        	default:
        	    return 'Unknown error';
        }
    }
}

    public static function decode($json)
    {
        $data = json_decode(self::clean($json), true);
        if (json_last_error() != JSON_ERROR_NONE) {
            throw new Exception\RuntimeException(sprintf(
                'Unable to decode a JSON string. The cause: "%s".',
                json_last_error_msg()
            ));
        }

        return $data;
    }
получилось не плохо.

Я согласен со всеми(участниками темы), первый пример ужасен, но чтот не смог сразу понять чем...

По сути если взять тело функции(и метода), есть три способа выйти раньше конца тела: return, throw + break выход из текущей конструкции(for, while, foreach, switch), по сути, если мы бросаем return или throw мы можем абсолютно наплевать, что цикл не закончен, что в конструкции switch например не пойдёт интерпретатор по следующим case`ам.

Просто не мог понять, что меня смущало, по сути мне был необходим результат, функция выполнилась(json_decode), дальше нужно проверить, что она корректно выполнилась, и если нет, выдать отформатированную ошибку.
А я поехал по стези того, что ошибка управляет дальнейшим результатом выполнения кода. Чистая ошибка бизнес логики.

флоппик, hell0w0rd, Вурдалак спасибо, что указали на это.

Но теперь хочу вернуться к PSR-2:
PHP:
switch ($expr) {
    case 0:
        echo 'First case, with a break';
        break;
    case 1:
        echo 'Second case, which falls through';
        // no break
    case 2:
    case 3:
    case 4:
        echo 'Third case, return instead of break';
        return;
    default:
        echo 'Default case';
        break;
}
это нормально? зачем они делают в конце break? зачем лишняя конструкция управления?
 

AmdY

Пью пиво
Команда форума
stalxed
продемонстрированный тобой код аналог GOTO, с break и без него
его проблема в том, что имеется несколько точек выхода, лучше заводить временнюу переменную и делать throw-return после switch, да и при дебаге-расширении будет удобнее.
 

Фанат

oncle terrible
Команда форума
PHP:
            throw new Exception\RuntimeException(sprintf(
                'Unable to decode a JSON string. The cause: "%s".',
                json_last_error_msg()
            ));
А меня вот эта конструкция бесит.
Абсолютно бессмысленное (т.е. не несущее никакого смысла) нагромождение кода.
Для чтения которого нужно затействовать стек.
Выкидывая по ходу мусорные операторы
PHP:
            $err = 'Unable to decode a JSON string. The cause: "'.json_last_error_msg().'".';
            throw new Exception\RuntimeException($err);
получается короче, но, главное - ЧИТАБЕЛЬНЕЕ.
Он читается последовательно: определили сообщение об ошибке, кинули с ним исключение.
Все просто и линейно.

Я понимаю эту моду, идущую от анонимных функций.
Но Ё Ж ТЫ МОЁ - сколько бы умных заклинаний вы не произнесли -PSR1, PSR2, PSR100500 - этот код как был говнокашей, так и останется.
Любой стандарт призван делать код более читабельным и поддерживаемым.
А тут мы имеем ДВЕ, МАТЬ ЕГО, СКОБКИ, РЯДОМ!
Что первая закрывает? А что вторая? А хрен его знает.
Причем две - это далеко не предел. Любители и десяток наструячат. Всё в полном соответствии со стандартами.

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

Фанат

oncle terrible
Команда форума
PHP:
		return [
            JSON_ERROR_DEPTH            => 'Maximum stack depth exceeded',
            JSON_ERROR_STATE_MISMATCH   => 'Underflow or the modes mismatch',
            JSON_ERROR_CTRL_CHAR        => 'Unexpected control character found',
            JSON_ERROR_SYNTAX           => 'Syntax error, malformed JSON',
            JSON_ERROR_UTF8             => 'Malformed UTF-8 characters, possibly incorrectly encoded'
		]
        [json_last_error()]
- ад, кстати, из той же серии. "Ой, в пхп ввели новые фишечки! Вот уж поиграемся!"
 

Вурдалак

Продвинутый новичок
Фанат, а для тебя всё, что незнакомо — ад. Мне кажется это как раз наиболее предпочтительным вариантом, нежели лепить static $errorMessageMap, который больше нигде, кроме этой функции, не нужен.
 

Вурдалак

Продвинутый новичок
получается короче, но, главное - ЧИТАБЕЛЬНЕЕ.
А мне кажется наоборот, ты ещё и ввёл мусорную $err.

Если отойти от исключений, то плейсхолдеры предпочтительней для той же i18n.

Сам по себе sprintf, конечно, хорошо бы заменить методом для string типа
PHP:
'Unexpected token "%s" at line %d'->format(':', 42)
но это ж PHP, такое лет через 10 увидим (и потребуется ещё лет 10, чтобы @Фанат принял это).
 
Последнее редактирование:

stalxed

Новичок
stalxed
продемонстрированный тобой код аналог GOTO, с break и без него
его проблема в том, что имеется несколько точек выхода, лучше заводить временнюу переменную и делать throw-return после switch, да и при дебаге-расширении будет удобнее.
В этом есть рациональное звено. Но почему должна минимизация точек выхода? Чем вот точка выхода из функции(метода) более уникальна, чем например точка выхода из цикла или switch?

Вурдалак написал(а):
Сам по себе sprintf, конечно, хорошо бы заменить методом для string типа
Было бы очень круто, но вроде потихоньку начинают http://www.php.net/manual/en/book.spl-types.php
Авось меньше 10 лет пройдёт.

ФАНАТ написал(а):
А тут мы имеем ДВЕ, МАТЬ ЕГО, СКОБКИ, РЯДОМ!
Что первая закрывает? А что вторая? А хрен его знает.
Вы ещё не программировали на языке смайликов(LISP), я сам его не люблю, экзамен как никак провалил по нему, помню... Вот там реально идёт голова кругом от кучи смай... скобочек.
И сложение строк я заменил на функцию sprintf, когда стал путаться в веренице точек(сложения строк), кавычек... spintf в коде гораздо удобнее выглядит.
Кстати саму идею использования spintf, честно говоря почерпнул из рекомендаций фреймворка Zend.
 

hell0w0rd

Продвинутый новичок
PHP:
class JsonConverter
{
    public function encode($value, $options = 0)
    {
        return json_encode($value, $options);
    }

    public function decode($json, $assoc = true, $depth = 512, $options = 0)
    {
        $result = json_decode($json, $assoc, $depth, $options);
        if (JSON_ERROR_NONE !== $err = json_last_error()) {
            throw new JsonConverterException($err);
        }

        return $result;
    }
}
PHP:
class JsonConverterException extends \RuntimeException
{
    private $errors = array(
        JSON_ERROR_DEPTH          => 'Maximum stack depth exceeded',
        JSON_ERROR_STATE_MISMATCH => 'Underflow or the modes mismatch',
        JSON_ERROR_CTRL_CHAR      => 'Unexpected control character found',
        JSON_ERROR_SYNTAX         => 'Syntax error, malformed JSON',
        JSON_ERROR_UTF8           => 'Malformed UTF-8 characters, possibly incorrectly encoded'
    );

    public function __construct($code)
    {
        if (array_key_exists($code, $this->errors)) {
            $message = $this->errors[$code];
        } else {
            $message = 'Undefined error';
        }
        parent::__construct($message, $code);
    }
}
сойдет?
Кстати, ТС, если ты черпаешь идеи из зенда, то уж обрати внимание, что на каждый класс там создано свое exception. А если у exception своя какая-то логика - то уж точно нужно его выносить в отдельный класс, а не городить все в одном
 
Последнее редактирование:

hell0w0rd

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

Фанат

oncle terrible
Команда форума
Офигеть логика.
В лиспе куча скобочек - поэтому здесь я тоже буду ставить.

Конкатенация его, видите ли, в трепет приводит, а вкладывать кучу функций друг в друга - нет.
 

Фанат

oncle terrible
Команда форума
Вурдалак
Поклонники стиля "фигак-фигак - и на продакшен" всегда были и будут. Ты не сказал ничего нового.
Собственно, народы некоторых стран целиком живут таким кодом.
 

Вурдалак

Продвинутый новичок
Для этого должны наконец забить на обратную совмеситимость
Так а где обратная совместимость-то ломается? :) Вот proof of concept.
Поклонники стиля "фигак-фигак - и на продакшен" всегда были и будут. Ты не сказал ничего нового.
Собственно, народы некоторых стран целиком живут таким кодом.
Я не понимаю о чём ты.
 
Сверху