Должны ли программисты использовать логические переменные для «документирования» своего кода? - PullRequest
78 голосов
/ 19 марта 2010

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

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

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

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

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

Ответы [ 14 ]

54 голосов
/ 19 марта 2010

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

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

16 голосов
/ 19 марта 2010

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

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

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

9 голосов
/ 19 марта 2010

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

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

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

bool deleteOnCompletion = true;

DoSomeMethod(deleteOnCompletion);
5 голосов
/ 19 марта 2010

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

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);
}

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

3 голосов
/ 19 марта 2010

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

//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 голосов
/ 19 марта 2010

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

2 голосов
/ 19 марта 2010

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

2 голосов
/ 19 марта 2010

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

2 голосов
/ 19 марта 2010

Делая это

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

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

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

bool ElementBlahBlah? (elementIndex, lastElementIndex);

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

1 голос
/ 23 марта 2010

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

bool success = false;

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

success = true;

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

return success;
...