Здравствуйте,

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

function CanSendMessageToUser(User $user)
{
    if(!$user->AccountEnabled)
    {
        return ...; // вызывающему коду нужно сообщить, что сообщение отправить нельзя потому что пользователь уже неактивен (заблокирован и т.п.)
    }
    else if(/* some other check */)
    {
        return ...; // вызывающему коду нужно сообщить, что сообщение отправить нельзя еще по какой-то причине
    }

    return ...; // сообщение можно отправить
}

У нас есть такие основные варианты:

1) Использовать исключения. Просто, мало кода, но почему-то мне кажется что не очень кошерно использовать тут исключения (наверное потому что исключения я привык использовать для каких-то исключительных ситуаций, которые не должны происходить в нормальном режиме работы).

2) Использовать коды ошибок и функцию для преобразования кода ошибки в текст:

function CanSendMessageToUser(User $user)
{
    if(!$user->AccountEnabled)
    {
        return SendMessageTestResult::ACCOUNT_DISABLED;
    }
    ...

    return SendMessageTestResult::SUCCESS;
}

class SendMessageTestResult
{
  const SUCCESS = 0;
  const ACCOUNT_DISABLED = 1;
  ...
}

function SendMessageErrorCodeToText($errorCode)
{
  switch($errorCode)
  {
    case SendMessageTestResult::ACCOUNT_DISABLED:
      return 'Cannot send message to the user because their account is disabled.';
    ...
  }
}

//Вызывающий код получается чистеньким:

$res = CanSendMessageToUser($user);
if($res !== SendMessageTestResult::SUCCESS)
  return Presenter::GetErrorMessage(SendMessageErrorCodeToText($res));

Минус такого варианта, в том что как-то много кода нам приходится писать (класс с константами кодов ошибок и функцию-конвертер кода ошибки в текст).

3) Возвращать константу SUCCESS или текст ошибки:

function CanSendMessageToUser(User $user)
{
    if(!$user->AccountEnabled)
    {
        return 'Cannot send message to the user because their account is disabled.';
    }
    ...

    return SUCCESS;
}

...

$res = CanSendMessageToUser($user);
if($res !== SUCCESS)
  return Presenter::GetErrorMessage($res);

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

4) Использовать специальный класс типа MethodResult, описывающий результат выполнения метода, и который может содержать код ошибки или описание ошибки:

function CanSendMessageToUser(User $user)
{
    if(!$user->AccountEnabled)
    {
        return MethodResult::ErrorMessage('Cannot send message to the user because their account is disabled.');
    }
    ...

    return MethodResult::Success();
}

...

$res = CanSendMessageToUser($user);
if(!$res->Success)
  return Presenter::GetErrorMessage($res->ErrorMessage);

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

---

Что скажут гуру дизайна и PHP?

Здравствуйте, MozgC, Вы писали:

MC>1) Использовать исключения. Просто, мало кода, но почему-то мне кажется что не очень кошерно использовать тут исключения

Т.е. ты предлагаешь свои видения обсудить? Или что?

Ответ на исходный вопрос — если в языке есть нормальные исключения, то следует использовать именно их.

Здравствуйте, AndrewVK, Вы писали:

AVK>Т.е. ты предлагаешь свои видения обсудить? Или что?
Свои видения я привел как возможные варианты. Интересует кто какой вариант бы выбрал (можно один из приведенных, можно еще какой-то свой) и почему.

AVK>Ответ на исходный вопрос — если в языке есть нормальные исключения, то следует использовать именно их.
Ясно.

Здравствуйте, MozgC, Вы писали:

MC>Здравствуйте,

MC>Допустим, нужно проверить, можно ли отправить сообщение пользователю, и если нет, вывести соответствующее сообщение с указанием причины (пример просто один из, т.е. могут быть другие похожие задачи и хочется выбрать типовое решения для использования в проекте).

Тут мне кажется надо определиться с таким моментом. Либо вы реализуете проверку и само действие по отдельности, либо проверку реализуете внутри действия.

Если проверка реализуется отдельно, то использование исключений тут не подходит, нужно в каком-либо виде возвращать результат проверки — строка, объект или ещё чего, это уже смотрите на средства вашего языка. Но при выполнении действия опять же придётся выполнять те же проверки, и если они не проходят, то тут уже нужно бросать исключение.

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

Т.е., в первом случае у вас будет CanSendMessageToUser, возвращающий ошибку и SendMessageToUser, который может выкинуть исключение.
Во втором случае — только SendMessageToUser, который в случае чего выкидывает исключение с указанием того, почему нельзя отправить сообщение.

На мой взгляд во многих случаях достаточно второго способа, без отдельной проверки. При выполнении действия просто вызывается нужный метод, возникающие исключения анализируются и на основании их выполняются какие-либо действия, чаще всего просто пользователю сообщается о проблеме.
Если требуется, например, блокировать пункт меню или кнопку когда действие недоступно, то добавляем метод CanХХХ, возвращающий логическое значение — можно ли выполнить действие или нет, без уточнения причины.

Вообще, можете посмотреть паттерн "Команда", он как раз подходит для подобных задач. Правда в классическом варианте он состоит из одного метода execute(), но ничто не мешает дополнить его и методом canExecute():Boolean.

Здравствуйте, MozgC, Вы писали:

MC>Здравствуйте,

MC>Допустим, нужно проверить, можно ли отправить сообщение пользователю, и если нет, вывести соответствующее сообщение с указанием причины (пример просто один из, т.е. могут быть другие похожие задачи и хочется выбрать типовое решения для использования в проекте).
MC>Представим такую функцию на PHP:

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

На примере посылки сообщения. Допустим, у пользователя есть ограничения бизнес-логики на отправку сообщения:
* максимальное количество сообщений в день;
* может посылать только тем пользователям, кто дал согласие.

При этом мы исходим из того, что пользователь должен быть авторизован в системе (глобальное предусловие).

Тогда контракт метода будет такой:

enum SendStatus {accepted, too_many_message_today, recepient_is_not_allow_message }

def sendMessage(userId, message) : SendStatus throws Exception

Т.е. в случае ошибки бизнес-логики мы вернем соответствующий статус, а в случае неожиданных проблем (пользователь не вошел в систему, поломалась связь с БД, поломалось ... ) прилетит исключение.

Клиент может обрабатывать исключение в одном месте, а статусы бизнес-логики в точках вызова метода сервиса. Например, если нет связи с БД, то эту ситуацию можно обрабатывать в каком-нибудь фильтре запросов до передачи данных в слой обработки бизнес-логики и показывать пользователю соответствующее сообщение.

Здравствуйте, AndrewVK, Вы писали:

AVK>Ответ на исходный вопрос — если в языке есть нормальные исключения, то следует использовать именно их.

В корне неверный ответ. Во-первых, исключения необходимо использовать только в исключительных ситуациях, т.е. в случае ошибки. На то они и исключения, чтобы уведомлять об ошибке. Для уведомления клиента функции о чем-то, что является нормальной ситуацией существует много разных методов. По сути все основные автор вопроса охватил. Функция называется CanSendMessageToUser, т.е. она проверяет возможность доставки сообщения к пользователю. Т.е. для нее как возможность доставки так и его отсутствие являются обычными нормальными ситуациями. Исключение здесь можно кидать только в случае, например, некорректной передачи аргумента User. Во всех остальных случаях получение исключения из данной функции — нонсенс (даже если вообще нет сети или еще невесть что происходит). Самый корректное возвращаемое значение — это bool. Если необходима расшифровка причины проблемы, то необходимо использовать последний способ предложенный автором. С точки зрения дальнейшей поддержки программы и простоты использования это самый корректный путь. А сходства с генерацией исключения тут нет. Когда возникает исключение, то подымается содержимое стека и проводится много еще другой доп. работы. Более того, если клиент этой процедуры не обернет вызов в try/catch то программа может вообще вылететь. А возврат объекта — это обычный возврат объекта.

Здравствуйте, Xenon_IPC, Вы писали:

X_I>В корне неверный ответ. Во-первых, исключения необходимо использовать только в исключительных ситуациях, т.е. в случае ошибки

А у ТС как раз ошибки, что характерно.

Здравствуйте, AndrewVK, Вы писали:

AVK>А у ТС как раз ошибки, что характерно.

Уважаемый AndrewVK, скажите как бы вы отреагировали на такую ситуацию: вот приходите Вы на собеседование, Вам дают листик с исходным кодом и спрашивают "имеются ли в этом коде ошибки?". Обнаружив такую в коде Вы начинаете с криками "Ааааа... КАРАУЛ! ОШИБКА!" метаться и бегать по комнате для собеседований, вместо того чтобы спокойно сказать, что да, ошибка есть и заключается она в том-то и том-то.
Исключительные ситуации это не средство информирования клиентов функций/библиотек о состоянии объектов, а средство оповещения что что-то в программе идет не так как запланировано и игнорировать это нельзя. В данном же случае предполагается обычная проверка на доступность удаленного клиента. Неужели то, что клиент прервал сессию это является ошибкой? Или даже если невозможность доставить сообщение происходит именно из-за ошибки в сети, то надо просто вернуть уведомление, что объект недоступен по такой-то причине.
Вообще, полезность данной функции для меня крайне сомнительна. Если она применяется для того, чтобы перед отправкой сообщения проверять можно ли это сделать (а судя по всему это так), то так делать нельзя, так как это чревато крайне редкими сбоями которые объяснить будет впоследствии невозможно. Все дело в том, что после проверки можно ли доставить сообщение удаленному клиенту и получение "утвердительного ответа" мы пытаемся это сделать, рассчитывая на то, что с клиентом все в порядке. Но за это время (после проверки и до начала доставки сообщения) с клиентом может что-то случиться вследствие чего доставить сообщение не удастся. Т.е. архитектурно верным решением было бы перенести эту проверку в саму функцию отправки. А вот в ней, уже в случае ошибки необходимо кидать именно исключения, потому что функция не может выполнить свое предназначение и игнорировать это нельзя и это именно как раз тот случай, где надо использовать исключения.

Здравствуйте, Xenon_IPC, Вы писали:

X_I>Уважаемый AndrewVK, скажите как бы вы отреагировали на такую ситуацию: вот приходите Вы на собеседование ...

Аналогии в качестве аргументов идут в лес, уважаемый Xenon_IPC.

X_I>Исключительные ситуации это не средство информирования клиентов функций/библиотек о состоянии объектов

В теме явно сказано: "Реализация функции проверяющей что-то и возвращающей ОК или ошибку". Еще раз указываю на сей интересный факт.

Здравствуйте, MozgC, Вы писали:

Можно использовать что-то вроде Maybe:

data Maybe a = Nothing | Just a

На PHP это может быть представлено просто в виде некой структуры, которая содержит два поля: Success и код ошибки. Соответственно, если Success===false, то код ошибки задан, всегда представлен однообразно (в виде целого числа), а текст ошибки можно получить через отдельную функцию, как вы и хотели.

Я часто использовал подобный подход. Суть в том, что часто на определенном уровне просто недостаточно "полномочий", чтобы сгенерировать исключение. Это актуально для всякого рода WCF сервисов, которые в случае ошибок должны генерировать исключения специального вида. Но часть логики сервиса может быть реализовано на уровне некой библиотеки, а генерировать WCF-ные исключения из библиотеки — несколько странно. Поэтому получается ситуация, что самописная библиотека генерирует исключения, эти исключения перехватываются, анализируются и потом, возможно, генерируются новые исключения. Такой подход, честно говоря, кажется немного неправильным. Особенно, когда вместо действительно "исключительно ситуации", мы имеем ошибку бизнес-логики.

Здравствуйте, Воронков Василий, Вы писали:

ВВ>На PHP это может быть представлено просто в виде некой структуры, которая содержит два поля: Success и код ошибки. Соответственно, если Success===false, то код ошибки задан, всегда представлен однообразно (в виде целого числа), а текст ошибки можно получить через отдельную функцию, как вы и хотели.

Я так понимаю это что-то среднее между приведенными мной вариантами 2 и 4. Мне в таком подходе не нравится что придется на каждую такую ситуацию писать enum с кодами ошибки и функцию получения текста ошибки по коду. Тогда мой вариант 4 лично для меня выглядит предпочтительнее — он позволяет просто получить текст ошибки, без написания дополнительной функции.

Благодарю всех за участие в теме!

Вообще судя по ответам в этой теме и по общению с другими опытными коллегами, в лидерах получаются варианты 1 и 4, т.е. либо генерация исключения, либо использование класса типа MethodResult { bool Success, int ErrorCode, string ErrorText }.

Так что я думаю есть смысл сузить вопрос до противостояния двух этих вариантов.

Против варианта с исключением есть такие основные аргументы:
1) Мы генерируем исключение в допустимой предвидимой ситуации, и это вроде как плохо.
2) Мы просто спрашиваем у метода CanSendMessageToUser() можно или нет, а он нам выкидывает исключение, что есть как-то неожиданно, неинтуитивно.

По поводу пункта 2 — проблема решается переименованием метода в EnsureMessageCanBeSent(), т.е. код может быть такой:

...
try
{
  EnsureMessageCanBeSent($user);
  // продолжаем действия по отправке сообщения, если во время них получится исключение - поймаем, залогируем и выведем его в top-level handler
}
catch(LogicException $ex)
{
  return UIHelper::GetErrorMessage('Message cannot be sent for the following reason: ' . $ex->getMessage());
}
...

По поводу пункта 1 хочется задать вопрос: а чем действительно на практике плохо, что мы сгенерируем исключение в такой ситуации?

PS. Я не за вариант с исключением, я пытаюсь лучше понять и выбрать между вариантом с иключением и вариантом 4 (возвратом MethodResult).

Здравствуйте, MozgC, Вы писали:

ВВ>>На PHP это может быть представлено просто в виде некой структуры, которая содержит два поля: Success и код ошибки. Соответственно, если Success===false, то код ошибки задан, всегда представлен однообразно (в виде целого числа), а текст ошибки можно получить через отдельную функцию, как вы и хотели.
MC>Я так понимаю это что-то среднее между приведенными мной вариантами 2 и 4. Мне в таком подходе не нравится что придется на каждую такую ситуацию писать enum с кодами ошибки и функцию получения текста ошибки по коду. Тогда мой вариант 4 лично для меня выглядит предпочтительнее — он позволяет просто получить текст ошибки, без написания дополнительной функции.

enum с кодами ошибок может быть весьма полезен и как своего рода спецификация — какие конкретно ошибки могут быть сгенерированы.
При желании вариант можно упростить — например, представлять ошибку всегда текстовым описанием. Но здесь уже зависит от конкретной ситуации. Код ошибки нужен, если он будет подвергаться машинному анализу, если нужно лишь показать пользователю ошибку или залогировать ее, и никакой анализ не предусматривается, можно возвращать сразу текст.

В хаскелле для таких вещей используется Either:

data Either a b = Left a | Right b

Left — это типа "левое" значение, например, ошибка. В нее как раз может быть завернут текст ошибки (а может, и код). Right — значит, что все в порядке, и в него завернут результат применения функции.

Здравствуйте, MozgC, Вы писали:

MC>По поводу пункта 1 хочется задать вопрос: а чем действительно на практике плохо, что мы сгенерируем исключение в такой ситуации?

Исключения (любые) плохи тем, что с исключениями результат применения функции зависит не только от ее аргументов, но и определяется неким контекстом, который является внешним по отношению к этой функции. Глядя на определение функции, невозможно понять, чем закончится исполнение это функции — вы можете из нее прыгнуть в другую функцию, в другой класс, другой модуль. Исключения — это такой goto на стероидах. Соответственно, они имеют все проблемы goto — да еще в степени. Поэтому во всех ситуациях, где возникает подозрение, что исключения начинают использоваться как control flow, хочется перестать их использовать.

Если проще — исключения вводят нондетерминизм там, где без него можно обойтись. Соответственно, код становится сложнее без явной на то необходимости.

Здравствуйте, MozgC, Вы писали:

Я бы разделил две задачи:
1. Классификацию ситуаций (+, возможнно, доп. информацию)
2. Вывод информации пользователю

Классификация: в самом простом случае у нас одно из двух: либо всё в порядке, либо есть проблема. В более интересном мы можем выделять несколько классов "серьёзности" ситуаций:
1. Ок, можно продолжать
2. Можно продолжать, но результат может оказаться не совсем ожидаемым ("сервер платежей партнёра недоступен, возможна задержка в зачислении денег на счёт")
3. Продолжить не получится, но ошибка имеет временный характер ("сервер недоступен, попробуйте позже")
4. Продолжить не получится, и с заданными параметрами не получится никогда ("вы не можете использовать дебетовую карту для подтверждения кредита")
Помимо "класса" серъёзности и конкретного "типа" ошибки, могут быть дополнительные параметры. Например, "не хватает места для копирования файла" может сообщать, каков размер файла, и сколько ещё не хватает.
Вывод информации пользователю осложнён тем, что проверочная функция изолирована от подробностей UI. Оформление и локализация явно должны определяться каким-то другим уровнем.

К сожалению, с PHP я не знаком, поэтому не знаю, какие вещи там делать удобно, а какие нет. В дотнете я бы возвращал экземпляр простой структуры, состоящей из Severity, Message, и Parameters. При этом Severity принимает одно из предопределённых значений, соответствующих уровню проблемы, а Message может начинаться с @, тогда это ключ локализации. Parameters — это object[]; скармливаются в String.Format вместе с локализованным текстом, чтобы выполнить подстановку значений.

Для удобства у структуры имеет смысл делать статический мембер OK, который возвращает "всё в порядке".

Ещё один нюанс: иногда имеет смысл выявлять более одной проблемы за раз. Тогда возвращать надо коллекцию результатов.

Здравствуйте, MozgC, Вы писали:

К сожалению ничего не могу сказать о PHP, так что все нижеописанное основано на опыте применения этого подхода в Windows Forms/WPF приложениях на основе MVVM.

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

// чтобы не заморачиваться с out-параметрами, делаем класс с результатами валидации
class ValidationResult
{
   public bool Success {get; private set;}
   public ReadonlyCollection<string> Errors {get;}
   public static ValidationResult FromErrors(IEnumerable<string> errors) {}
   // может быть куча других фабричных и вспомогательных методов, для объединения ошибок
   // для создания этого зверя по исключению и т.д.
}

// ViewModel или Model
public ValidationResult Validate()
{
    Func<Model, string>[] validationRules = 
    {
        model => model.AccountEnabled ? "User is not active" : String.Empty,
        model => model.AccountLocked ? "Users account is locked" : String.Empty,
    };

    Model m = new Model();
    var errors = validationRules
        .Select(f => f(m)) // запускаем валидацию
        .Where(s => !string.IsNullOrEmpty(s)) // получаем только реальные ошибки
        .ToList(); // триггерим выполнение
    return ValidationResult.FromErrors(errors);
}

public void Save()
{
   var validationResult = Validate();
   if (!validatioResult.Success)
     throw new InvalidOperationException(validationResult.Message);
  
  // выполняем логику сохранения
}

По-сути, я применяю обычно подход чем-то схожий с контрактами: у нас есть метод Save у которого есть предусловие: валидность состояния объекта. Вызывающий код вправе выяснить валидность текущего состояния перед выполнением операции с помощью метода Validate, который вернет соответствующий результат (обычно в виде некоторого объекта, поскольку булевыми флагами и строками оперировать не удобно, а бросать исключения в методах предварительной проверки состояния кажется не гуд). Либо, вызывающий код может сразу же начать выполнять некоторую операцию, но в этом случае он уже получит исключение, поскольку будет попытка выполнить операцию для невалидного состояния объекта.

Согласно контрактам, метод проверки предусловия должен быть доступен клиенту класса, но поскольку здесь речь идет не о контрактах, а о конкретной задаче, то метод валидации может быть закрытым.

Плюсы подхода:
1. Класс ValidationResult прячет в себе низкоуровневые подробности обработки ошибок. Например, туда довольно легко вставляется локализация; легко унифицировать разную модель обработки ошибок с нижнего уровня (вполне реально, что на нижнем уровне может быть зоопарк технологий по обработке ошибок, коды возврата, строки, исключения).
2. Класс ValidationResult легко расширить и добавить приоритеты ошибок: критикал, предупреждение и т.д. (о чем советовал Sinclair), у меня, например, этого нет, но добавить это просто.
3. Класс ValidationResult весьма просто представляет собой как одну ошибку, так и несколько.
4. Простота тестирования; метод Validate проще тестировать, чем метод Save, бросающий исключение.
5. Не захламляется логика основного метода. Валидация сама по себе — не такая и простая операция, так что она может занимать приличный объем. Уже это само по себе требует выделения этой активности в отдельный метод, с этим же "паттерном" это происходит истественным образом.
6. Использование функционального подхода в методе Validate тоже прикольная штука, которая хорошо читается и расширяется.

Здравствуйте, MozgC, Вы писали:

MC>Здравствуйте,

MC>Допустим, нужно проверить, можно ли отправить сообщение пользователю, и если нет, вывести соответствующее сообщение с указанием причины (пример просто один из, т.е. могут быть другие похожие задачи и хочется выбрать типовое решения для использования в проекте).
MC>Представим такую функцию на PHP:

В пхп я "не але!" но мне кажется почему-то это предельно простой вещью. В строго типизированном языке можно сделать так

UserAuthorization GetUserAuthorization (User usr)
{
    return new UserAuthorization(....);
}

В ПХП это наверное тогда будет или enum или еще что-то. Исключения кидаются когда сущность не может выполнить контракт. Т.е. MessageSender'у говорят отправь, а он бросает исключение, потому что не может отправить (у пользователя нет прав это делать).
Ситуация автора отличается от той в которой необходимо применять исключение. Он заранее может узнать можно ли отправлять или нет, и это валидное состояние для пользователя. Автор также модифицировать UI таким образом, что модель UI не будет слать в доменную модель отправку сообщения, вместо этого в UI будет восклицательный знак и хинт о том, что сообщение отправить нельзя, потому что прав нет, и т.д.