C ++ чистота кода

Question

Я работаю в среде C ++ и:
a) Нам запрещено использовать исключения
b) Это код сервера приложений / данных, который оценивает множество запросов различных типов

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

class OpResult
{
  .....
  bool succeeded();
  bool failed(); ....
  ... data error/result message ...
};

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

....
OpResult result = some_(mostly check)function(....);
if (result.failed())
  return result;
...

Вопрос в том, является ли плохой практикой делать макрос похожим на это и использовать его повсюду?

#define RETURN_IF_FAILED(call) \
  {                            \
    OpResult result = call;    \
    if (result.failed())       \
      return result;           \
  }

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

Answer 1

Это компромисс.Вы торгуете размером кода для обфускации логики.Я предпочитаю сохранять логику видимой.

Мне не нравятся макросы этого типа, потому что они нарушают Intellisense (в Windows) и отлаживают логику программы.Попробуйте установить точку останова для всех 10 return операторов в вашей функции - не проверка, а return.Попробуйте пройтись по коду, который находится в макросе.

Самое худшее в этом - то, что, как только вы это примете, трудно поспорить с макросами монстров из 30 строк, которые некоторые программисты ЛЮБЯТ использовать для широко используемых мини-приложений.задачи, потому что они «проясняют вещи».Я видел код, где различные типы исключений обрабатывались таким образом четырьмя каскадными макросами, в результате чего в исходном файле было 4 строки, причем макросы фактически расширялись до> 100 реальных строк.Теперь, вы уменьшаете раздувание кода?Нет. Невозможно легко сказать с помощью макросов.

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

РЕДАКТИРОВАТЬ: Один комментарий, который я должен добавить, заключается в том, что если вы действительно повторяете эту логику проверки ошибок снова и снова, возможно,в коде есть возможности рефакторинга.Не гарантия, а лучший способ уменьшения раздувания кода, если это применимо.Ищите повторяющиеся последовательности вызовов и инкапсулируйте общие последовательности в свои собственные функции, а не обращайтесь к тому, как каждый вызов обрабатывается изолированно.

Answer 2

На самом деле, я предпочитаю немного другое решение. Дело в том, что результат внутреннего вызова не обязательно является допустимым результатом внешнего вызова. Например, внутренняя ошибка может быть «файл не найден», а внешняя «конфигурация недоступна». Поэтому я предлагаю воссоздать OpResult (потенциально упаковывая в него "внутренний" OpResult для лучшей отладки). Это все идет в направлении «InnerException» в .NET.

технически, в моем случае макрос выглядит как

#define RETURN_IF_FAILED(call, outerresult) \
  {                                         \
    OpResult innerresult = call;            \
    if (innerresult.failed())               \
    {                                       \
        outerresult.setInner(innerresult);  \
        return outerresult;                 \
    }                                       \
  }

Это решение требует управления памятью и т. Д.

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

---

Мое мнение таково, что такие макросы не запутывают логику программы, а наоборот делают ее чище. С таким макросом вы объявляете свое намерение ясным и кратким способом, в то время как другой способ кажется слишком многословным и поэтому подверженным ошибкам. Заставить сопровождающих анализировать, что та же самая конструкция OpResult r = call(); if (r.failed) return r тратит их время.

Альтернативный подход без ранних возвратов заключается в применении к каждой строке кода шаблона, подобного CHECKEDCALL(r, call) с #define CHECKEDCALL(r, call) do { if (r.succeeded) r = call; } while(false). На мой взгляд, это намного хуже и определенно подвержено ошибкам, так как люди, как правило, забывают добавить CHECKEDCALL() при добавлении дополнительного кода.

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

Answer 3

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

// something.cpp

#define RETURN_IF_FAILED() /* ... */

void f1 () { /* ... */ }
void f2 () { /* ... */ }

#undef RETURN_IF_FAILED

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

Answer 4

Есть ли какая-либо информация в результате, которая действительно полезна в случае сбоя вызова?

Если нет, то

static const error_result = something;

if ( call().failed() ) return error_result; 

будет достаточно.

Answer 5

По моему мнению, скрывать оператор возврата в макросе - плохая идея.«Обфукация кода» (мне нравится этот термин ...!) Достигает максимально возможного уровня.Мое обычное решение таких проблем - агрегировать выполнение функции в одном месте и контролировать результат следующим образом (при условии, что у вас 5 нулевых функций):

std::array<std::function<OpResult ()>, 5>  tFunctions = {
 f1, f2, f3, f4, f5
};

auto tFirstFailed = std::find_if(tFunctions.begin(), tFunctions.end(), 
    [] (std::function<OpResult ()>& pFunc) -> bool {
        return pFunc().failed();
    });

if (tFirstFailed != tFunctions.end()) {
 // tFirstFailed is the first function which failed...
}

Answer 6

Может быть сделано с C ++ 0x лямбдами.

template<typename F> inline OpResult if_failed(OpResult a, F f) {
    if (a.failed())
        return a;
    else
        return f();
};

OpResult something() {
    int mah_var = 0;
    OpResult x = do_something();
    return if_failed(x, [&]() -> OpResult {
        std::cout << mah_var;
        return f;
    });
};

Если вы умны и отчаянны, вы можете заставить трюк такого же типа работать с обычными объектами.

Answer 7

Я согласен с POV Стива .

Я сначала подумал, хотя бы уменьшить макрос до

#define RETURN_IF_FAILED(result) if(result.failed()) return result;

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

---

Я думаю, в основном, вы должны сделать компромисс между способностью записи и читабельностью. Макрос определенно проще записать . Однако остается открытым вопрос, проще ли также читать . Последнее довольно субъективное суждение. Тем не менее, объективное использование макросов делает запутанным кодом.

---

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