PHP несколько if / elseif и сообщения об ошибках / лучшие практики

Question

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

Как лучше всего выложить, если необходимо проверить операторы, возвраты, сообщения и т. Д., Если требуется проверить 3, 4, 5 или более различных условий и установить либо сообщение об ошибке, либо обработка продолжается. Рекомендуется ли физически проверять все ошибки в начале кода?

Вот пример с некоторыми условиями типа реального мира.

function process($objectId,$userId,$newData)
{
    $error = '';
    if(($object = $this->getObject($objectId)) && $object->userOwnsObject($userId))
    {
        if($this->isValid($newData))
        {
            if($object->isWriteable())
            {
                if($object->write($newData))
                {
                    // No error. Success!
                }
                else
                {
                    $error = 'Unable to write object';
                }
            }
            else
            {
                $error = 'Object not writeable';
            }
        }
        else
        {
            $error = 'Data invalid';
        }
    }
    else
    {
        $error = 'Object invalid';
    }
    return $error;
}

OR

function process($objectId,$userId,$newData)
{
    $error = '';
    if((!$object = $this->getObject($objectId)) && !$object->userOwnsObject($userId))
    {
        $error = 'Object invalid';
    }
    elseif(!$this->isValid($newData))
    {
        $error = 'Data invalid';
    }
    elseif(!$object->isWriteable())
    {
         $error = 'Object not writeable';
    }
    elseif(!$object->write($newData))
    {
        $error = 'Unable to write to object';
    }
    else
    {
        // Success!
    }
    return $error;
}

Мне ясно, что в этом случае вариант 2 - это путь. Это намного понятнее. Теперь мы можем сделать его немного сложнее:

function process($objectId,$userId,$newData)
{
    $error = '';
    if(($object = $this->getObject($objectId)) && $object->userOwnsObject($userId))
    {
        $this->setValidationRules();
        $parent = $object->getParentObject();
        $parent->prepareForChildUpdate();

        if($this->isValid($newData,$parent))
        {
            $newData = $this->preProcessData($newData);

            if($object->isWriteable())
            {
                // doServerIntensiveProcess() has no return value and must be done between these two steps
                $this->doServerIntensiveProcess();

                if($object->write($newData))
                {
                    // No error. Success!
                    $parent->childUpdated();
                }
                else
                {
                    $error = 'Unable to write object';
                }
            }
            else
            {
                $error = 'Object not writeable';
            }
        }
        else
        {
            $error = 'Data invalid';
        }
    }
    else
    {
        $error = 'Object invalid';
    }
    return $error;
}

ИЛИ с некоторыми проблемами

function process($objectId,$userId,$newData)
{
    $error = '';
    if((!$object = $this->getObject($objectId)) && !$object->userOwnsObject($userId))
    {
        $error = 'Object invalid';
    }
    // Is it wrong to hate multi-line conditionals?
    elseif(!$this->setValidationRules() || (!$parent = $object->getParentObject()) ||
        !$parent->prepareForChildUpdate() || !$this->isValid($newData,$parent))
    {
        $error = 'Data invalid';
    }
    elseif((!$newData = $this->preProcessData($newData)) || !$object->isWriteable())
    {
         $error = 'Object not writeable';
    }
    // Where does doServerIntensiveProcess() with no return value go??
    elseif(!$object->write($newData))
    {
        $error = 'Unable to write to object';
    }
    else
    {
        // Success!
         $parent->childUpdated();
    }
    return $error;
}

Я просто не уверен, что лучше всего справиться с такими вложенными функциональными возможностями типа «если это то потом делать, то потом, если это так делать». Спасибо вам за ваше понимание!

Answer 1

То, что я стараюсь поддерживать в чистоте кода, выглядит следующим образом:

function process($objectId,$userId,$newData)
{
    $object = $this->getObject($objectId);

    if($object === false)
    {
        return "message";
    }

    if($object->userOwnsObject($userId) === false)
    {
        return "message";
    }

    if($this->setValidationRules() === false)
    {
        return "unable to set validation rules";
    }

    if(false !== ($parent = $object->getParentObject()))
    {
        return "unable to get parent object";
    }

    /*... etc ...*/

    //if your here the all the checks above passed.
}

, делая это таким образом, вы также экономите ресурсы, так как вы непосредственно возвращаетесь на место, код выглядит чище и не нужно2 много гнезд

, но если вы строите функцию с нуля, я не понимаю, почему вы не можете использовать исключения в вашем новом коде, это не будет мешать текущему приложению и упрощает работу

function process($objectId,$userId,$newData)
{    
    if(false !== ($parent = $object->getParentObject()))
    {
          throw Exception("unable to get parent object");
    }

    /*... etc ...*/
}

и

try
{
    $this->process(....);
}
catch(Exception $e)
{
    show_error_page('invalid.php',$e);
}

или другим способом является создание класса обработки ошибок с помощью статического метода с именем InternalError, например

abstract class Error
{
    public static InternalError(Exception $Ex)
    {
        Logger::LogException($Ex);
        //Then flush all buffers and show internal error,
    }
}

, а не show_error_pageвыше вы можете сделать:

try
{
    $this->process(....);
}
catch(Exception $e)
{
    Error::InternalError($e); //this provides user with an interface to report the error that has just been logged.
}

таким образом все ваши Exception регистрируются и могут просматриваться в вашей системе администрирования, что означает, что вы можете быстрее отслеживать ошибки и не полагаться на то, что участники видят ошибки, но получите хорошее извинение с формой электронной почты с просьбой описать, что они пытались сделать, идентификатор ошибки будет прикреплен к форме, чтобы вы могли отследить пользователя до ошибки.

ThЭто лучшая форма обработки ошибок IMO.

Answer 2

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

Answer 3

Я бы полностью пропустил

if($object->isWriteable())
{
    if($object->write($newData))
    {
        // ..
    }
}

И выдавал исключения при вызове object-write () (без возвращаемого значения).То же самое и для других примеров

if ($something->isValid($data)) {
    $op->doSomething($data); // throws XyException on error
}

Если вы действительно хотите использовать такие конструкции, вы также можете использовать swtch -Statement

switch (true) {
    case !$something->isValid($data):
        $errors = "error";
        break;
    // and so an
}

Но я очень рекомендуюИсключения.

Answer 4

Как насчет этого?

function process($objectId,$userId,$newData)
{
    if((!$object = $this->getObject($objectId)) && !$object->userOwnsObject($userId))
        return 'Object invalid';
    elseif(!$this->isValid($newData))
        return 'Data invalid';
    elseif(!$object->isWriteable())
        return 'Object not writeable';
    elseif(!$object->write($newData))
        return 'Unable to write to object';

    // Success!
}

Для более сложного примера ::

function process($objectId,$userId,$newData)
{
    if(!($object = $this->getObject($objectId)) || !$object->userOwnsObject($userId))
        return 'Object invalid';

    $this->setValidationRules();
    $parent = $object->getParentObject();
    $parent->prepareForChildUpdate();

    if(!$this->isValid($newData,$parent))
        return 'Data Invalid';

    $newData = $this->preProcessData($newData);

    if(!$object->isWriteable())
        return 'Object not writable';

    // doServerIntensiveProcess() has no return value and must be done between these two steps
    $this->doServerIntensiveProcess();

    if(!$object->write($newData))
        return 'Unable to write object';

    // No error. Success!
    $parent->childUpdated();
    return '';
}