Какой стиль кодирования лучше?
https://stackoverflow.com/questions/1641156
-
08-07-2019 - |
italiano
english
français
española
中国
日本の
العربية
Deutsch
한국어
Português
RussianВопрос
Во время проверки кода старший разработчик прокомментировал некоторые вложения, которые я использовал в своем коде.Он предложил мне установить значение bool таким образом, чтобы у меня никогда не было более одного уровня вложенности.Я думаю, что мой код более читабелен, но хочу узнать мнение других разработчиков по этому поводу.Какой стиль лучше?Обосновано ли его непроизвольное отвращение к гнездованию?
Ниже приведены некоторые упрощенные примеры кода.
Вложенный:
If(condition1)
{
If(condition2)
{
if(condition3)
{
return true;
}
else
{
log("condition3 failed");
}
else
{
log("condition2 failed")
}
}
else
{
log("condition1 failed")
}
return false;
или
Управляемый Bool:
bool bRC = false;
bRC = (condition1);
if(brc)
{
bRC = (condition2);
}
else
{
log("condition1 failed");
return false;
}
if(bRC)
{
bRC = (condition3);
}
else
{
log("condition2 failed");
return false;
}
if(bRC)
{
return true;
}
else
{
log("condition3 failed");
return false;
}
Решение
Мне больше нравится твой, но я бы, наверное, сделал что-то вроде:
if (condition1 && condition2 && condition3)
{
return true;
}
else if (!condition1)
{
log("condition1 failed");
}
else if (!condition2)
{
log("condition2 failed");
}
else
{
log("condition3 failed");
}
return false;
Если условия являются сложными выражениями, то я мог бы присвоить выражения переменным с соответствующими именами перед вычислением операторов if, чтобы избежать необходимости пересчитывать значения в каждом if.
Это предполагает, что обычный режим заключается в том, что все условия выполняются, и поэтому вы хотите сначала выполнить эту проверку.Если обычный режим заключается в том, что одно или несколько условий являются ложными, то я бы изменил порядок их выполнения и проверил каждое отрицание по очереди и просто вернул true, если все проверки завершились неудачей.Это также устранило бы необходимость в том, чтобы временные переменные заменяли сложные выражения.
Другие советы
Если у вас нет никаких глупых правил о нескольких точках возврата, я думаю, что это довольно приятно (как и у кого-то другого, но они удалили свой ответ по неизвестным причинам):
if(!condition1)
{
log("condition1 failed");
return false;
}
if(!condition2)
{
log("condition2 failed");
return false;
}
if(!condition3)
{
log("condition3 failed");
return false;
}
return true;
Может быть, это такое же непроизвольное отвращение к супер-вложенности, но это определенно чище, чем его дерьмо, хранящее логические условия в определенных значениях.Однако это может быть менее читабельным в контексте:подумайте, было ли одно из условий isreadable().Так понятнее сказать if(isreadable()) потому что мы хотим знать, доступно ли что-то для чтения. if(!isreadable()) предполагает, хотим ли мы знать, является ли это недоступным для чтения, что не входит в наши намерения.Конечно, спорно, что могут быть ситуации, когда одна из них более читабельна / интуитивно понятна, чем другая, но я сам поклонник этого способа.Если кто-то зациклится на возвратах, вы могли бы сделать это:
if(!condition1)
log("condition1 failed");
else if(!condition2)
log("condition2 failed");
else if(!condition3)
log("condition3 failed");
else
return true;
return false;
Но это довольно коварно и, на мой взгляд, менее "понятно".
Лично я нахожу вложенный код значительно более легким для чтения.
Обычно я предпочитаю nested для своих условий;конечно, если мои вложенные условия отступают слишком далеко вправо, я должен начать задаваться вопросом, есть ли лучший способ сделать то, что я пытаюсь сделать (рефакторинг, редизайн и т.д.)
Похоже на вложенную версию, но намного чище для меня:
if not condition1:
log("condition 1 failed")
else if not condition2:
log("condition 2 failed")
else if not condition3:
log("condition 3 failed")
else
return true;
return false;
Помните, что каждое условие оценивается один раз.
Второй стиль абсурдно многословен:действительно ли он предложил именно это?Большинство из них вам не нужны if утверждений, потому что существует return в другой части.
С вложенным примером вы полагаетесь на то, чтобы не забыть включить все возможные else пункты.
Ни то, ни другое не кажется мне удовлетворительным.
Способ, управляемый bool, сбивает с толку.Вложенность хороша, если требуется, но вы могли бы уменьшить часть глубины вложенности, объединив условия в один оператор или вызвав метод, в котором выполняется некоторая дальнейшая оценка.
Я думаю, что оба пути возможны и имеют свои плюсы и минусы.Я бы использовал стиль, основанный на bool, в тех случаях, когда у меня была бы действительно глубокая вложенность (8 или 10 или что-то в этом роде).В вашем случае с 3 уровнями я бы выбрал ваш стиль, но для точного примера, приведенного выше, я бы выбрал так:
void YourMethod(...)
{
if (condition1 && condition2 && consition3)
return true;
if (!condition 1)
log("condition 1 failed");
if (!condition 2)
log("condition 2 failed");
if (!condition 3)
log("condition 3 failed");
return result;
}
...или что-то в этом роде, если вы предпочитаете единственную точку выхода (как это делаю я)...
void YourMethod(...)
{
bool result = false;
if (condition1 && condition2 && consition3)
{
result = true;
}
else
{
if (!condition 1)
log("condition 1 failed");
if (!condition 2)
log("condition 2 failed");
if (!condition 3)
log("condition 3 failed");
}
return result;
}
Таким образом, вы получите все сбои, зарегистрированные при первом запуске.В вашем примере вы получите только одно зарегистрированное условие сбоя, даже если существует более одного условия сбоя.
Я бы, наверное, пошел с
if (!condition1) log("condition 1 failed");
else if (!condition2) log("condition 2 failed");
else if (!condition3) log("condition 3 failed");
// -------------------------------------------
return condition1 && condition2 && condition3;
что, я считаю, равноценно и намного чище...
Кроме того, как только клиент решит, что все условия должны быть оценены и занесены в журнал в случае их сбоя, а не только первое, которое не выполняется, это гораздо проще изменить, чтобы сделать это:
if (!condition1) log("condition 1 failed");
if (!condition2) log("condition 2 failed");
if (!condition3) log("condition 3 failed");
// -------------------------------------------
return condition1 && condition2 && condition3;
Вот как я бы это реализовал, при условии, что ваши реализации действительно отражают желаемое поведение.
if (!condition1) {
log("condition1 failed");
return false;
}
if (!condition2) {
log("condition2 failed");
return false;
}
if (!condition3) {
log("condition3 failed");
return false;
}
return true;
Однако я думаю, что более вероятно, что каждое сбойное условие должно регистрироваться.
result = true;
if (!condition1) {
log("condition1 failed");
result = false;
}
if (!condition2) {
log("condition2 failed");
result = false;
}
if (!condition3) {
log("condition3 failed");
result = false;
}
return result;
Если язык поддерживает обработку исключений, я бы выбрал следующее:
try {
if (!condition1) {
throw "condition1 failed";
}
if (!condition2) {
throw "condition2 failed";
}
if (!condition3) {
throw "condition3 failed";
}
return true;
} catch (e) {
log(e);
return false;
}
ПРАВКА От Чарльза Бретаны:Пожалуйста, посмотрите Использование исключений для потока управления
Мне не нравится ни то, ни другое.Когда у вас так много гнезд, что-то не так.В случае проверки формы или чего-то еще, что действительно требует чего-то подобного, попробуйте придумать что-то более модульное или компактное.
Примером может служить массив, содержащий условия, с помощью которых вы будете выполнять итерацию некоторое время и печатать / прерывать по мере необходимости.
Существует слишком много реализаций в зависимости от ваших потребностей, поэтому создание примера кода было бы бессмысленным.
Как правило, когда ваш код выглядит слишком сложным, это полный отстой :).Попробуй переосмыслить это.Следование методам кодирования в большинстве случаев делает код намного более эстетичным и коротким;и, очевидно, еще и умнее.
Код должен переформулировать проблему на заданном языке.Поэтому я утверждаю, что любой из фрагментов может быть "лучше".Это зависит от моделируемой проблемы.Хотя я предполагаю, что ни одно из решений не будет соответствовать реальной проблеме.Если вы поставите реальные условия вместо condition1,2, 3, это может полностью изменить "лучший" код.
Я подозреваю, что есть лучший (3d) способ написать все это вместе.
if( condition1 && condition2 && condition3 )
return true;
log(String.Format("{0} failed", !condition1 ? "condition1" : (!condition2 ? "condition2" : "condition3")));
return false;
Таким образом, вам не нужно видеть много строк кода только для ведения журнала.И если все ваши условия верны, вы не тратите время на их оценку на случай, если вам придется войти в систему.
