Иногда мне кажется, что легче проверить, истинны ли все условия, но тогда обрабатывать только "другую" ситуацию.
Я думаю, иногда мне кажется, что легче узнать, что что-то действительно, и предположить, что все остальные случаи недействительны.
Например, скажем, что нам действительно очень важно, когда что-то не так:
object value = GetValueFromSomeAPIOrOtherMethod();
if((value != null) && (!string.IsNullOrEmpty(value.Prop)) && (possibleValues.Contains(value.prop)))
{
// All the conditions passed, but we don't actually do anything
}
else
{
// Do my stuff here, like error handling
}
Или просто я должен изменить это:
object value = GetValueFromSomeAPIOrOtherMethod();
if((value == null) || (string.IsNullOrEmpty(value.Prop)) || (!possibleValues.Contains(value.prop)))
{
// Do my stuff here, like error handling
}
Или (что я считаю уродливым):
object value = GetValueFromSomeAPIOrOtherMethod();
if(!((value != null) && (!string.IsNullOrEmpty(value.Prop)) && (possibleValues.Contains(value.prop))))
{
// Do my stuff here, like error handling
}
Хотя для меня редко, иногда я чувствую, что запись в этой форме приводит к самому четкому коду в некоторых случаях. Пойдите для формы, которая обеспечивает самую ясность. Компилятору все равно, и он должен генерировать по существу (возможно, точно) тот же код.
Однако может быть яснее определить логическую переменную, которой присваивается условие в инструкции if(), а затем записать ваш код как отрицание этой переменной:
bool myCondition = (....);
if (!myCondition)
{
...
}
Наличие пустого блока if с инструкциями в else - это просто плохой стиль. Извините, это одно из моих любимых раздражителей. Нет ничего функционально неправильного в этом, это просто заставляет мои глаза кровоточить.
Просто! выпишите инструкцию if и поместите там свой код. IMHO уменьшает шум и делает код более удобочитаемым.
Я должен предисловие к этому, сказав, что это мое личное предпочтение, но я, как правило, вынимаю логику проверки из кода и в свою собственную функцию проверки. В этот момент ваш код становится намного "опрятным", просто говоря:
if(!ValidateAPIValue(value))
Это, на мой взгляд, кажется намного более кратким и понятным.
Просто использование части else неприемлемо. Вам не нужно беспокоиться о применении правила Де-Моргана, просто не все высказывание. То есть, от if (cond) до if (!(cond)).
Я думаю, что это совершенно неприемлемо.
Единственная причина - избежать единственного отрицания и пары круглых скобок вокруг выражения. Я согласен с тем, что выражения в вашем примере ужасны, но они непримиримо запутаны для начала! Разделите выражение на части приемлемой ясности, сохраните их в логических точках (или сделайте из них методы) и объедините их, чтобы сделать свое условие if-statement.
Одна подобная конструкция, которую я часто использую, уходит рано. Я не пишу код следующим образом:
if (validityCheck1)
{
if (validityCheck2)
{
// Do lots and lots of things
}
else
{
// Throw some exception, return something, or do some other simple cleanup/logic (version 2)
}
}
else
{
// Throw some exception, return something, or do some other simple cleanup/logic. (version 1)
}
Вместо этого я пишу это:
if (!validityCheck1)
{
// Throw some exception, return false, or do some other simple logic. (version 1)
}
if (!validityCheck2)
{
// Throw some exception, return false, or do some other simple logic. (version 2)
}
// Do lots and lots of things
Это имеет два преимущества:
Только несколько случаев ввода недопустимы, и они имеют простую обработку. Их нужно немедленно обработать, чтобы как можно скорее вывести их из нашей ментальной модели и полностью сосредоточиться на важной логике. Особенно, если в вложенных операторах есть несколько проверок валидности.
Блок кода, который обрабатывает действительные случаи, обычно будет самой большой частью метода и содержит собственные вложенные блоки. Это намного менее загромождено, если этот блок кода сам не вложен (возможно, несколько раз) в if-statement.
Таким образом, код более читабельен и легче рассуждать.
Извлеките свои условия, затем вызовите
if(!ConditionsMetFor(value))
{
//Do Something
}
Хотя это не всегда удобно, я обычно предпочитаю изменять
if (complexcondition){} else {/*stuff*/}
to
if (complexcondition) continue;
/*stuff*/
(или выйти с return, break и т.д.). Конечно, если условие слишком сложное, вы можете заменить его несколькими условиями, из-за которых код вырывается из того, что он делает. Это в основном относится к проверке и проверке ошибок типов кода, где вы, вероятно, захотите выйти, если что-то пойдет не так.
Если я вижу "if", я ожидаю, что он что-то сделает.
if(!condition)
является более читаемым.
if(condition) {
//do nothing
}
else {
//do stuff
}
по существу гласит: "Если мое условие выполнено, ничего не делайте, иначе что-нибудь сделайте".
Если мы хотим прочитать ваш код в качестве прозы (который хороший, самодокументирующий код должен быть доступен для чтения таким образом), который просто слишком многословен и вводит больше концепций, чем необходимо для достижения вашей цели. Прикрепите "!".
Ваш вопрос похож на мой ответ (упрощение условий) на любимый программист, незнакомый с домашним животным
Для языков, которые не поддерживают до построения, цепочка нескольких NOTs делает наши глаза кровоточащими.
Какой из них легче читать?
Это:
while (keypress != escape_key && keypress != alt_f4_key && keypress != ctrl_w_key)
Или это:
until (keypress == escape_key || keypress == alt_f4_key || keypress == ctrl_w_key)
Я придерживаюсь мнения, что последнее легче пробить, чем первое. Первый из них включает слишком много NOT, а условия AND делают логику более липкой, она заставляет вас читать все выражение, прежде чем вы сможете убедиться, что ваш код действительно правильный, и это будет намного больше сложнее читать, если ваша логика включает сложную логику (влечет зацепление большего количества И, очень липкое).
В колледже теорема Де Моргана преподается в нашем классе. Я очень благодарен за то, что логику можно упростить, используя его теорему. Поэтому для языковой конструкции, которая не поддерживает до утверждения, используйте это:
while !(keypress == escape_key || keypress == alt_f4_key || keypress == ctrl_w_key)
Но так как C не поддерживает скобки без указания while/if, нам нужно добавить скобки на наш код DeMorgan'd:
while (!(keypress == escape_key || keypress == alt_f4_key || keypress == ctrl_w_key))
И что, возможно, побудило Дэна С комментировать, что код DeMorgan'd больше болит его глазами на мой ответ на любимый программист, незнакомый с домашним животным
Но действительно, код DeMorgan'd легче читать, чем иметь несколько NOTS и липких AND
[EDIT]
Ваш код (DeMorgan'd):
object value = GetValueFromSomeAPIOrOtherMethod();
if ( value == null || string.IsNullOrEmpty(value.Prop)
|| !possibleValues.Contains(value.prop) )
{
// Do my stuff here, like error handling
}
.. отлично. Фактически, то, что большинство программистов (особенно из языков, которые не имеют try/catch/окончательно строят из get-go), делают, чтобы убедиться, что условия выполнены (например, использование нулевых указателей, использование правильных значений и т.д.), перед продолжением операций.
Примечание. Я воспользовался свободой удаления лишних скобок на вашем коде, возможно, вы пришли с языка Delphi/Pascal.
Это плохой стиль, рассмотрим несколько очень полезных альтернатив:
Использовать стиль предложения охраны:
object value = GetValueFromSomeAPIOrOtherMethod();
if((value != null) && (!string.IsNullOrEmpty(value.Prop)) && (possibleValues.Contains(value.prop)))
{
return;
}
// do stuff here
Извлеките условный в свой собственный метод, это сохраняет логичность и легкость чтения:
bool ValueHasProperty(object value)
{
return (value != null) && (!string.IsNullOrEmpty(value.Prop)) && (possibleValues.Contains(value.prop));
}
void SomeMethod()
{
object value = GetValueFromSomeAPIOrOtherMethod();
if(!ValueHasProperty(value))
{
// do stuff here
}
}
Я делаю это, когда мой мозг может легко обернуться вокруг логики успеха, но громоздко понять логику отказа.
Я обычно просто комментирую "//no op", поэтому люди знают, что это не ошибка.
Это не очень хорошая практика. Если вы использовали ruby, вы бы сделали:
unless condition
do something
end
Если ваш язык этого не позволяет, вместо выполнения
if(a){}else{something}
делать
if(!a){something}
Я считаю, что это неприемлемо (хотя я уверен, что я сделал это в прошлом), чтобы иметь пустой блок. Это означает, что что-то нужно делать.
Я вижу, что другие вопросы говорят о том, что это более читаемый второй способ. Лично я говорю, что ни один из ваших примеров не является особенно читаемым. Приведенные вами примеры выпрашивают метод IsValueValid (...).
Я иногда оказываюсь в связанной, но немного другой ситуации:
if ( TheMainThingIsNormal () )
; // nothing special to do
else if ( SomethingElseIsSpecial () ) // only possible/meaningful if ! TheMainThingIsNormal ()
DoSomethingSpecial ();
else if ( TheOtherThingIsSpecial () )
DoSomethingElseSpecial ();
else // ... you see where I'm going here
// and then finish up
Единственный способ вынуть пустой блок - создать больше вложенности:
if ( ! TheMainThingIsNormal () )
{
if ( SomethingElseIsSpecial () )
DoSomethingSpecial ();
else if ( TheOtherThingIsSpecial () )
DoSomethingElseSpecial ();
else // ...
}
Я не проверяю условия исключения или проверки - я просто забочусь о специальных или разовых случаях, поэтому я не могу просто выручить раньше.
Мой ответ обычно не будет... но я думаю, что хороший стиль программирования основан на согласованности.....
поэтому, если у меня есть много выражений, которые выглядят как
if (condition)
{
// do something
}
else
{
// do something else
}
Затем случайный "пустой", если блок является точным, например.
if (condition)
{ } // do nothing
else
{
// do something else
}
Причиной этого является то, что если ваши глаза видят что-то несколько раз, их менее вероятно замечать изменение, например. крошечный "!". Поэтому, несмотря на то, что это плохо, что нужно делать изолированно, его вероятность того, что кто-то будет поддерживать код в будущем, поймет, что это конкретное, если... а... отличается от остальных...
Другой конкретный scenerio, где он может быть приемлемым, относится к какой-то логике конечного автомата, например.
if (!step1done)
{} // do nothing, but we might decide to put something in here later
else if (!step2done)
{
// do stuff here
}
else if (!step3done)
{
// do stuff here
}
Это явно подчеркивает последовательный поток состояний, шаги, выполняемые на каждом (даже если это ничего). Id предпочитает его над чем-то вроде...
if (step1done && !step2Done)
{
// do stuff here
}
if (step1done && step2done && !state3Done)
{
// do stuff here
}
Мне нравится вторая версия. Это делает код более чистым. На самом деле это одна из вещей, которые я бы попросил исправить во время обзора кода.
Я всегда стараюсь и реорганизую большие условия, подобные этому, в свойство или метод, для удобства чтения. Итак:
object value = GetValueFromSomeAPIOrOtherMethod();
if((value == null) || (string.IsNullOrEmpty(value.Prop)) || (!possibleValues.Contains(value.prop)))
{
// Do my stuff here, like error handling
}
становится примерно таким:
object value = GetValueFromSomeAPIOrOtherMethod();
if (IsValueUnacceptable(value))
{
// Do my stuff here, like error handling
}
...
/// <summary>
/// Determines if the value is acceptable.
/// </summary>
/// <param name="value">The value to criticize.</param>
private bool IsValueUnacceptable(object value)
{
return (value == null) || (string.IsNullOrEmpty(value.Prop)) || (!possibleValues.Contains(value.prop))
}
Теперь вы всегда можете повторно использовать метод/свойство, если это необходимо, и вам не нужно слишком много думать о методе потребления.
Конечно, IsValueUnacceptable, вероятно, будет более конкретным именем.
первый:
object value = GetValueFromSomeAPIOrOtherMethod();
var isValidValue = (value != null) && (!string.IsNullOrEmpty(value.Prop)) && (possibleValues.Contains(value.prop));
if(!isValidValue)
{
// Do my stuff here, like error handling
}
2cnd:
object value = GetValueFromSomeAPIOrOtherMethod();
if(!isValidAPIValue(value))
{
// Do my stuff here, like error handling
}
Все ли выражения одинаковы? В языках, поддерживающих короткое замыкание, изменение между ансами и орками может быть фатальным. Помните && использовать в качестве охранника, чтобы другие условия не проверялись даже.
Будьте осторожны при конвертировании. Есть больше ошибок, чем вы ожидали.
В этих случаях вы можете пожелать абстрагировать логику проверки в самом классе, чтобы помочь не загромождать код вашего приложения.
Например
class MyClass
{
public string Prop{ get; set; }
// ... snip ...
public bool IsValid
{
bool valid = false;
if((value != null) &&
(!string.IsNullOrEmpty(value.Prop)) &&
(possibleValues.Contains(value.prop)))
{
valid = true
}
return valid;
}
// ...snip...
}
Теперь ваш код приложения
MyClass = value = GetValueFromSomewhere();
if( value.IsValie == false )
{
// Handle bad case here...
}
Я поклонник DeMorgan Rule, который принимает ваш ex3 и производит ex2. Пустое, если блок является ментальным блоком imo. Вы должны перестать читать ничего, что существует, - тогда вам нужно задаться вопросом, почему.
Если вам нужно оставлять комментарии, например//Это просто пустое; то код не очень понятен.
Стиль, который следует за тем, чтобы один пустой блок if-else считался плохим стилем. для хорошей практики программирования, если вам не нужно писать, если блок вам нужно поставить (!) "Не", если заблокировать. Не нужно писать еще
Если (условие) // пустой еще // код
можно заменить как
если (! Состояние) // код
это также сохранение дополнительной строки кода.
Я бы не сделал этого в С#. Но я делаю это в Python, потому что у Python есть ключевое слово, которое означает "ничего не делать":
if primary_condition:
pass
elif secondary_condition1:
do_one_thing()
elif secondary_condition2:
do_another_thing()
Можно сказать, что { } функционально эквивалентен pass, что и есть. Но это не (для человека) семантически эквивалентно. pass означает "ничего не делать", в то время как для меня { } обычно означает "там был код здесь, а теперь нет".
Но в целом, если я дойду до того, что даже проблема, связанная с тем, чтобы придерживаться ! перед условием, усложняет чтение, у меня проблема. Если я нахожу, что пишу код следующим образом:
while (keypress != escape_key && keypress != alt_f4_key && keypress != ctrl_w_key)
мне довольно ясно, что то, что я на самом деле захочу в долгосрочной перспективе, больше похоже на это:
var activeKeys = new[] { escape_key, alt_f4_key, ctrl_w_key };
while (!activeKeys.Contains(keypress))
потому что это делает явным концепцию ( "эти ключи активны" ), которая только неявна в предыдущем коде, и делает логику "это то, что вы делаете, когда неактивный ключ нажат" вместо "это то, что происходит, когда ключ, на который не нажимается ни один ESC, ALT + F4 или CTRL + W."