Я читаю Завершенный Код McConell, и он обсуждает логические переменные использования для документирования кода. Например, вместо:
if((elementIndex < 0) || (MAX_ELEMENTS < elementIndex) ||
(elementIndex == lastElementIndex)){
...
}
Он предлагает:
finished = ((elementIndex < 0) || (MAX_ELEMENTS < elementIndex));
repeatedEntry = (elementIndex == lastElementIndex);
if(finished || repeatedEntry){
...
}
Это кажется мне логической, хорошей практикой, и очень самодокументирующий. Однако я не решаюсь начинать использовать эту технику регулярно, поскольку я почти никогда не сталкивался с нею; и возможно это сбивало бы с толку только на основании того, чтобы быть редким. Однако мой опыт еще не очень обширен, таким образом, я интересуюсь слушанием мнения программистов об этой технике, и мне было бы любопытно знать, использует ли кто-либо регулярно эту технику или часто видел его при чтении кода. Действительно ли это - стоящая конвенция/стиль/техника принять? Другие программисты будут понимать и ценить его или считать его странным?
Единственный способ увидеть, что это идет не так, - это если у логического фрагмента нет имени, которое имеет смысл, а имя все равно выбрано.
//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-критериев, состоящих более чем из трех частей» - обычное дело »только для получения комментариев, которые семантически пусты и относятся к следующему типу
i++; //increment i by adding 1 to i's previous value
Лично я считаю, что это отличная практика. Это влияние на выполнение кода минимально, но ясность, которую он может обеспечить при правильном использовании, неоценима, когда дело доходит до поддержки кода в дальнейшем.
Разделение слишком вложенного и сложного выражения на более простые подвыражения, присваиваемые локальным переменным, а затем снова собираемые вместе, является довольно распространенным и популярным приемом - независимо от того, являются ли подвыражения и/или общее выражение булевыми или любого другого типа. При хорошо подобранных именах, со вкусом выполненная декомпозиция такого рода может повысить читабельность, и хороший компилятор без проблем сгенерирует код, эквивалентный исходному сложному выражению.
Некоторые языки, в которых нет понятия "присваивание" как такового, такие как Haskell, даже вводят специальные конструкции, позволяющие использовать технику "присвоения имени подвыражению" (клаузула where
в Haskell) - кажется, это говорит о некоторой популярности рассматриваемой техники
Я редко создаю отдельные переменные. Что я делаю, когда тесты становятся сложными, так это вложение IF и добавление комментариев. Like
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)
...
Признанный недостаток этой техники в том, что следующий программист, который придет, может изменить логику, но не потрудиться обновить комментарии. Думаю, это общая проблема, но у меня было много случаев, когда я видел комментарий, в котором говорилось "подтвердить идентификатор клиента", а следующая строка проверяла номер детали или что-то подобное, и я оставался в недоумении, где здесь идентификатор клиента.
По моему опыту, я часто возвращался к некоторым старым сценариям и задавался вопросом: «Что, черт возьми, я тогда думал?». Например:
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;
};
Я думаю, что лучше создавать функции/методы вместо временных переменных. Так читаемость повышается еще и потому, что методы становятся короче. В книге Мартина Фаулера "Рефакторинг" есть хорошие советы по улучшению качества кода. Рефакторинг, относящийся к вашему конкретному примеру, называется "Заменить Temp на Query" и "Извлечь метод".
Делая это
finished = ((elementIndex < 0) || (MAX_ELEMENTS < elementIndex));
repeatedEntry = (elementIndex == lastElementIndex);
if(finished || repeatedEntry){
...
}
вы удаляете логику из своего мозга и помещаете ее в код. Теперь программа знает, что вы имели в виду.
Всякий раз, когда вы называете что-то, вы даете этому физическое представление. Оно существует.
Вы можете манипулировать и использовать его повторно.
Вы даже можете определить весь блок как предикат:
bool ElementBlahBlah? (elementIndex, lastElementIndex);
и делать больше вещей (позже) в этой функции.
Я стараюсь делать это везде, где возможно. Конечно, вы используете «лишнюю строку» кода, но в то же время вы описываете, почему вы сравниваете два значения.
В вашем примере я смотрю на код и спрашиваю себя: «Хорошо, почему человек, который видит значение меньше 0?» Во втором вы ясно говорите мне, что некоторые процессы завершаются, когда это происходит. Во втором не догадываешься, каковы были твои намерения.
Самым большим для меня является то, что я вижу такой метод, как: DoSomeMethod (true);
Почему он автоматически устанавливается на true? Это гораздо более читабельно, например
bool deleteOnCompletion = true;
DoSomeMethod(deleteOnCompletion);
Если выражение сложное, я либо перемещаю его в другую функцию, которая возвращает bool
, например, isAnEveningInThePubAGoodIdea (dayOfWeek, sizeOfWorkLoad, amountOfSpareCash)
, либо пересматриваю код, чтобы сложное выражение не требуется.
Я использовал его, хотя обычно булеву логику оборачивали в метод многократного использования (если вызывается из нескольких мест).
Это способствует удобочитаемости, и когда логика меняется, ее нужно изменить только в одном месте.
Другие поймут это и не сочтут это странным (за исключением тех, кто когда-либо писал только тысячи строчных функций, то есть).
Я думаю, это зависит от того, какой стиль предпочитаете вы/ваша команда. "Ввести переменную" рефакторинг может быть полезен, но иногда нет :)
И я должен не согласиться с Кевином в его предыдущем посте. Его пример, я полагаю, можно использовать в случае, когда введенная переменная может быть изменена, но вводить ее только для одного статического буля бесполезно, потому что у нас есть имя параметра в объявлении метода, так зачем дублировать его в коде?
например:
void DoSomeMethod(boolean needDelete) { ... }
// useful
boolean deleteOnCompletion = true;
if ( someCondition ) {
deleteOnCompletion = false;
}
DoSomeMethod(deleteOnCompletion);
// useless
boolean shouldNotDelete = false;
DoSomeMethod(shouldNotDelete);
Помните, что таким образом вы вычисляете больше, чем необходимо. Из-за извлечения условий из кода вы всегда вычисляете их оба (без короткого замыкания).
Итак, что:
if((elementIndex < 0) || (MAX_ELEMENTS < elementIndex) ||
(elementIndex == lastElementIndex)){
...
}
После преобразования становится:
if((elementIndex < 0) || (MAX_ELEMENTS < elementIndex) |
(elementIndex == lastElementIndex)){
...
}
В большинстве случаев это не проблема, но все же в некоторых случаях это может означать снижение производительности или другие проблемы, например когда во втором выражении вы предполагаете, что первое не удалось.
Приведенный пример:
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);
}
Это имеет свою цену, конечно, это два дополнительных метода. Если вы будете делать это часто, это может сделать ваш код более читабельным, но ваши классы будут менее прозрачными. Но опять же, вы также можете переместить дополнительные методы во вспомогательные классы.
, если метод требует уведомления об успехе: (примеры в C #) Мне нравится использовать
bool success = false;
для начала. код будет ошибочным, пока я не изменю его на:
success = true;
, а затем в конце:
return success;