Следует ли программистам использовать логические переменные для «документирования» своего кода?

79

Я читаю Код Макконелла , и он обсуждает использование логических переменных для документирования вашего кода. Например, вместо:

if((elementIndex < 0) || (MAX_ELEMENTS < elementIndex) || 
   (elementIndex == lastElementIndex)){
       ...
}

Он предлагает:

finished = ((elementIndex < 0) || (MAX_ELEMENTS < elementIndex));
repeatedEntry = (elementIndex == lastElementIndex);
if(finished || repeatedEntry){
   ...
}

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

фроди
источник
@Paul R - упс, спасибо. Я начал с самодокументирования и понял, что для этого нет тега, поэтому попытался изменить его на самодокументирование, которое уже существует. :)
froadie
Разве не лучше просто комментировать происходящее в тесте?
JS
3
Когда это уместно, я тоже стараюсь сделать это, особенно если мне нужно проверять условия в разных частях метода. Это тоже гораздо более наглядно.
geffchang
«Это кажется мне логичным, передовым и очень самодокументированным». Судя по вашему опыту, скорее всего, это будет так же легко понято другими программистами, которые столкнутся с этим впервые.
2
Почему этот вопрос все еще открыт?
Orangepips

Ответы:

55

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

Некоторые языки, в которых отсутствует концепция «присваивания» как таковая, например, Haskell, даже вводят специализированные конструкции, позволяющие использовать технику «дать имя части выражения» ( whereпредложение в Haskell) - кажется, некоторые популярность рассматриваемой техники! -)

Алекс Мартелли
источник
6
если это проще И легче читать, я говорю, что это довольно ясный случай беспроигрышного
варианта
Я согласен. Во многих случаях вам, вероятно, сойдет с рук короткий комментарий, но я не понимаю, как это может повредить.
Max E.
16

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

Это способствует удобочитаемости, и когда логика меняется, ее нужно изменить только в одном месте.

Другие поймут это и не сочтут это странным (то есть, за исключением тех, кто когда-либо писал только тысячи строчных функций).

Одед
источник
Спасибо за ответ! Что касается методов многократного использования, у них есть еще одна (не связанная) веская причина для исключения ... Итак, я полагаю, мой вопрос действительно заключается в том, следует ли исключить одноразовые логические выражения, когда нет другой причины, кроме удобочитаемости. (Что, конечно, само по себе является достаточно серьезной причиной :)) Спасибо, что указали на это.
froadie
+1 для уменьшения количества изменений кода, необходимых при изменении логики, и насмешек над программистами, использующими тысячу строк.
Джеффри Л. Уитледж,
9

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

В вашем примере я смотрю на код и спрашиваю себя: «Хорошо, почему человек, который видит, что значение меньше 0?» Во втором вы ясно говорите мне, что некоторые процессы завершаются, когда это происходит. Во втором не догадываешься, каковы были твои намерения.

Самым большим для меня является то, что я вижу такой метод, как: DoSomeMethod(true); Почему он автоматически устанавливается на true? Это намного читаемее, например

bool deleteOnCompletion = true;

DoSomeMethod(deleteOnCompletion);
kemiller2002
источник
7
Я не люблю логические параметры именно по этой причине. В конечном итоге вы получаете такие вызовы, как «createOrder (true, false, true, true, false)» и что это означает? Я предпочитаю использовать перечисления, поэтому вы говорите что-то вроде «createOrder (Source.MAIL_ORDER, BackOrder.NO, CustomOrder.CUSTOM, PayType.CREDIT)».
Джей
Но если вы последуете примеру Кевина, он эквивалентен вашему. Какая разница, может ли переменная принимать 2 значения или более 2?
Марк Рузон,
2
С Jay's вы можете быть более ясным в определенных случаях. Например, при использовании PayType. Если бы это было логическое значение, параметр, вероятно, получил бы имя isPayTypeCredit. Вы не знаете, что это за альтернатива. С помощью перечисления вы можете четко увидеть, какие параметры PayType: Кредит, Чек, Наличные, и выбрать правильный.
kemiller2002
++ также перечисление не позволяет присваивать значение null, поэтому управление значениями в буквальном смысле завершено, а автоматическая документация перечисления наверху
Hardryv
Objective-C и Smalltalk действительно решают эту проблему в Objective-C:[Object createOrderWithSource:YES backOrder:NO custom:YES type:kCreditCard];
Грант Пол
5

Предоставленный образец:

finished = ((elementIndex < 0) || (MAX_ELEMENTS < elementIndex));
repeatedEntry = (elementIndex == lastElementIndex);
if(finished || repeatedEntry){
   ...
}

Также можно переписать для использования методов, которые улучшают читаемость и сохраняют логическую логику (как указал Конрад):

if (IsFinished(elementIndex) || IsRepeatedEntry(elementIndex, lastElementIndex)){
   ...
}

...

private bool IsFinished(int elementIndex) {
    return ((elementIndex < 0) || (MAX_ELEMENTS < elementIndex));
}

private bool IsRepeatedEntry(int elementIndex, int lastElementIndex) {
    return (elementIndex == lastElementIndex);
}

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

Prutswonder
источник
Если вы столкнулись с большим количеством шума кода с C #, вы также можете воспользоваться преимуществами частичных классов и переместить шум в частичные, и если люди заинтересованы в том, что проверяет IsFinished, легко перейти к ним.
Крис Марисич,
3

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

//No clue what the parts might mean.
if(price>0 && (customer.IsAlive || IsDay(Thursday)))

=>

first_condition = price>0
second_condition =customer.IsAlive || IsDay(Thursday)

//I'm still not enlightened.
if(first_condition && second_condition)

Я указываю на это, потому что правила типа «комментируйте весь свой код», «используйте именованные логические значения для всех if-критериев с более чем 3 частями» - обычное дело »только для получения комментариев, которые семантически пусты следующего вида

i++; //increment i by adding 1 to i's previous value
Мэтью Мартин
источник
2
Вы неправильно сгруппировали условия в своем примере, не так ли? '&&' связывается сильнее, чем '||' на большинстве языков, которые их используют (за исключением сценариев оболочки).
Джонатан Леффлер,
Добавлены родители. Так что это было бы еще одним ударом против разделения на именованные переменные, это дает возможность изменить группировку неочевидным образом.
MatthewMartin
2

Делая это

finished = ((elementIndex < 0) || (MAX_ELEMENTS < elementIndex));
repeatedEntry = (elementIndex == lastElementIndex);
if(finished || repeatedEntry){
   ...
}

вы удаляете логику из своего мозга и вставляете ее в код. Теперь программа знает, что вы имели в виду.
Когда вы что-то называете, вы даете этому физическое представление. Это существует.
Вы можете манипулировать им и использовать его повторно.

Вы даже можете определить весь блок как предикат:

bool ElementBlahBlah? (elementIndex, lastElementIndex);

и сделайте больше (позже) в этой функции.

Ник Дандулакис
источник
И что еще более важно, следующий разработчик, который взглянет на код, тоже поймет, что вы имели в виду! Это отличная практика, и я делаю это все время.
Крис Торнтон,
1
Кроме того, следующим разработчиком часто может быть вы, который снова смотрит на код через несколько месяцев (или даже несколько недель) и рад, что он хорошо задокументирован.
Louis
2

Если выражение сложное, то я либо перемещаю его в другую функцию, которая возвращает, boolнапример isAnEveningInThePubAGoodIdea(dayOfWeek, sizeOfWorkLoad, amountOfSpareCash),, либо пересматриваю код, чтобы такое сложное выражение не требовалось.

Бенедикт Коэн
источник
2

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

Так что:

if((elementIndex < 0) || (MAX_ELEMENTS < elementIndex) || 
   (elementIndex == lastElementIndex)){
   ...
}

После преобразования становится:

if((elementIndex < 0) || (MAX_ELEMENTS < elementIndex) |
   (elementIndex == lastElementIndex)){
   ...
}

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

Конрад Гарус
источник
Верно - я просто заметил это сейчас, когда вернулся к части моего кода, чтобы провести рефакторинг пары операторов if с использованием этого метода. Был оператор if, использующий оценку короткого замыкания с помощью && (вторая часть выдает NPE, если первая часть ложна), и мой код не работал, когда я реорганизовал это (потому что он всегда оценивал оба и сохранял их в логическом переменные). Хорошее замечание, спасибо! Но когда я пробовал это, мне было интересно - есть ли способ сохранить логику в переменной и отложить оценку до фактического оператора if?
froadie
2
Я действительно сомневаюсь, что компилятор разрешит это. Если второй вызов неэффективен, это обычно связано с вызовом какой-либо функции, и, как ни странно, компилятор не пытается определить, не имеет ли вызываемая функция побочных эффектов.
JS
Вы можете вложить IF и не выполнять последующие вычисления, если только первого теста недостаточно, чтобы решить, следует ли продолжать.
Джей
@froadie Некоторые языки, такие как Kotlin (а ​​вскоре и Dart) допускают "ленивые" переменные, которые будут вычисляться только при использовании. В качестве альтернативы, размещение логики в функции вместо переменной будет иметь тот же эффект. Просто, понимаете, на случай, если вы все еще захотите узнать это через 10 лет.
hacker1024
2

Думаю, лучше вместо временных переменных создавать функции / методы. Таким образом повышается удобочитаемость еще и потому, что методы становятся короче. В книге Мартина Фаулера «Рефакторинг» есть хорошие советы по повышению качества кода. Рефакторинг, связанный с вашим конкретным примером, называется «Заменить временный запрос на запрос» и «Метод извлечения».

мкдж
источник
2
Вы хотите сказать, что, загромождая пространство классов множеством разовых функций, вы улучшаете читаемость? Пожалуйста, объясни.
Zano
Это всегда компромисс. Читаемость исходной функции улучшится. Если исходная функция короткая, она того не стоит.
mkj
Кроме того, я думаю, что «загромождение пространства классов» зависит от используемого языка и того, как вы разбиваете свой код.
mkj
2

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

РычаниеСобака
источник
1

если для метода требуется уведомление об успехе: (примеры в C #) Мне нравится использовать

bool success = false;

чтобы начать. код неверен, пока я не изменю его на:

success = true;

затем в конце:

return success;
Крис Хейс
источник
0

Думаю, это зависит от того, какой стиль предпочитаете вы / ваша команда. Рефакторинг "ввести переменную" может быть полезен, но иногда нет :)

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

например:

void DoSomeMethod(boolean needDelete) { ... }

// useful
boolean deleteOnCompletion = true;
if ( someCondition ) {
    deleteOnCompletion = false;
}
DoSomeMethod(deleteOnCompletion);

// useless
boolean shouldNotDelete = false;
DoSomeMethod(shouldNotDelete);
дчекмарев
источник
0

По моему опыту, я часто возвращался к некоторым старым сценариям и задавался вопросом: «Что, черт возьми, я тогда думал?». Например:

Math.p = function Math_p(a) {
    var r = 1, b = [], m = Math;
    a = m.js.copy(arguments);
    while (a.length) {
        b = b.concat(a.shift());
    }
    while (b.length) {
        r *= b.shift();
    }
    return r;
};

что не так интуитивно понятно, как:

/**
 * An extension to the Math object that accepts Arrays or Numbers
 * as an argument and returns the product of all numbers.
 * @param(Array) a A Number or an Array of numbers.
 * @return(Number) Returns the product of all numbers.
 */
Math.product = function Math_product(a) {
    var product = 1, numbers = [];
    a = argumentsToArray(arguments);
    while (a.length) {
        numbers = numbers.concat(a.shift());
    }
    while (numbers.length) {
        product *= numbers.shift();
    }
    return product;
};
Rolandog
источник
-1. Если честно, это имеет мало общего с исходным вопросом. Вопрос в другом, очень конкретном, а не в том, как вообще писать хороший код.
П Швед
0

Я редко создаю отдельные переменные. Когда тесты усложняются, я вкладываю IF и добавляю комментарии. подобно

boolean processElement=false;
if (elementIndex < 0) // Do we have a valid element?
{
  processElement=true;
}
else if (elementIndex==lastElementIndex) // Is it the one we want?
{
  processElement=true;
}
if (processElement)
...

Признанный недостаток этой техники состоит в том, что следующий приходящий программист может изменить логику, но не утруждает себя обновлением комментариев. Я предполагаю, что это общая проблема, но я много раз видел комментарий, в котором говорится: «Подтвердите идентификатор клиента», а в следующей строке проверяется номер детали или что-то подобное, и мне остается задаться вопросом, где покупатель входит id.

Джей
источник