23/03/2021

Плохой пример (1): «Advanced» for loop в стиле «senior developer»

В этой небольшой заметке я предлагаю рассмотреть два варианта описания несложного алгоритма. Несложного алгоритма поиска некоторого признака по определённым условиям. Здесь я предлагаю оценить качество двух описаний с точки зрения поддержки — понимания человеком, а не с точки зрения эффективности (скорости выполнения, использования ресурсов). Именно поэтому я использую термин «описание» (может даже стоит назвать это «написанием»), так как алгоритм используется один и тот же — взаимный перебор элементов массива со значениями и элементов массива с условиями. Результат заносится в массив Boolean [] Results. Представьте себе, что вы встречаете следующий участок кода:
for (Integer Counter1 = 0, Flag = 1; Counter1 < Total; Results[Counter1] = (Flag != 0), Counter1++, Flag = 1)
  for (Integer Counter2 = 0; Counter2 < Total - 1; Counter2++)
    if (ConditionArray[Counter2] == Counter1)
      Flag = 0;
Сейчас задумайтесь, сколько времени вам понадобилось бы чтобы понять этот участок? Понять в степени достаточной для того, чтобы вы могли внести коррективу — найти и исправить ошибку, изменить условия. А без вводных данных, что я дал в начале статьи? А если бы вы встретили где-то такой фрагмент, как вы могли бы описать его с тем, чтобы облегчить понимание его другими людьми, или вами же в будущем? Здесь оговоримся, что я обезличил названия переменных и названия счётчиков, массивов и прочих сущностей на практике могут быть несколько более осмысленными.
Рассмотрим «нормальное» написание этого алгоритма. Которое я реализовал в первую очередь, так как это написание первым и приходит в голову (может быть не только мне).
// Проходим по всем элементам:
for (Integer Counter1 = 0; Counter1 < Total; Counter1++)
{
  // Ищем совпадение условий:
  for (Integer Counter2 = 0; Counter2 < Total - 1; Counter2++)
  {
    // Сбрасываем флаг в значение по-умолчанию:
    Results[Counter2] = true;
    if (ConditionArray[Counter2] == Counter1)
    {
      // Выставляем флаг и прерываем цикл. В ситуации, которая меня 
      // привела к написанию этой статьи, дальнейший перебор не имел смысла:
      Results[Counter2] = false;
      break;
    }
  }
}
Всё понятно (на сколько вообще код может быть понятен) и комментарии органично влились в код.
Так откуда вообще взялся фрагмент кода, с которого я начал статью? Я сделал его сам. Как я пришёл к этому? Я начал устранять лишние скобки (как я люблю делать после завершения реализации участка кода). Увлёкшись этим процессом, я вспомнил о том, что цикл for состоит из трёх блоков. Таким образом, перенося по частям алгоритм в эти блоки, я и получил «компактное» представление этого цикла.
Теперь поясню, как я адаптировал «нормальное» написание в ненормальное. Ещё раз сокращённый код (чуть понятнее представил блоки цикла):
for
    (
     Integer Counter1 = 0, Flag = 1;
     Counter1 < Total; 
     Results[Counter1] = (Flag != 0), Counter1++, Flag = 1
    )
  for
      (
        Integer Counter2 = 0;
        Counter2 < Total - 1;
        Counter2++
      )
    if (ConditionArray[Counter2] == Counter1)
      Flag = 0;
Здесь запись флага условия в результирующий массив перенесена в третий блок внешнего цикла for. Конструкция «!= 0» используется для приведения Integer к Boolean, точнее для получения Boolean из Integer'а. По правилам большинства популярных языков программирования false — это ноль, всё остальное — true. Соответственно, выражение «!= 0» даёт true. Для адаптации алгоритма к сокращённому написанию, я применил конструкцию, не являющуюся самой понятной для человека. Впрочем, здесь можно было бы использовать любой «маркер» для передачи признака выполнения искомого условия, тем самым ещё больше запутать описание алгоритма. Почему здесь Integer? Потому что в первом блоке цикла for — блоке инициализации, могут использоваться только переменные одного типа, поэтому мне пришлось работать с флагом как с Integer'ом, смешав его с переменной-счётчиком. В противном случае, мне пришлось бы выносить инициализацию флага за пределы верхнего (первого) цикла. Так же в третий блок перенесён сброс значения флага.
Отметим, что комментирование такого написания алгоритма крайне затруднено. Тут скорее нужно не построчные пояснения, а целый документ, описывающий логику реализованного кода.
Для ещё большего снижения читаемости проверку Condition можно было бы заменить на тернарный оператор, но здесь он не подходит, так как он является конструкцией типа if-then-else и выставляет флаг в ненужное положение в блоке else. В ситуации, где он подошёл бы, можно было бы заставить код выглядеть ещё чуть «круче».
Хотя я писал в начале статьи, что мы здесь не будем рассматривать вопросы эффективности, можно добавить, что компактное исполнение будет работать чуть хуже полноценного. Всё потому, что в полноценном варианте перебор заканчивается (break) при нахождении условия, а в компактном цикл проходит до конца — есть избыточные операции. Эта разница специфична для задачи, в ситуации, где придётся проходить весь массив в поисках всех условий (Condition) этой разницы в производительности не будет.
Завернув нормальное исполнение в компактное, я ужаснулся, тому, что получилось, представив себе, что мне когда-то придётся здесь что-то изменить. На получение нормального, понятного написания потребовалось некоторое время. И в итоге, я заново написал «обычное» написание, мне было проще так поступить, чем «разворачивать» компактный вариант обратно.

Выводы
Простой алгоритм настолько естественно выглядящий, с точки зрения языка и компилятора, оказывается в высокой степени трудно поддерживаемым для человека и даже непригодным к описанию (комментированию). Это говорит нам о том, что нужно писать не максимально «крутой» код, а максимально понятный и легко поддерживаемый.

No comments:

Post a Comment