Попался любопытный фрагмент кода, который должен реализовать копирование из одного двумерного массива в другой:
let selection = null; const func1 = (s, e) => { switch (e.command) { case 'paste': { destSelection = s.getSelection(); sourceSelection = { ...selection }; do { i = 0; do s.SetCellValue(destSelection.leftColumnIndex + (i++), destSelection.topRowIndex, s.getCellValue(sourceSelection.leftColumnIndex, sourceSelection.topRowIndex) ?? ''); while (sourceSelection.rightColumnIndex > sourceSelection.leftColumnIndex++); destSelection.topRowIndex++; sourceSelection.leftColumnIndex = selection.leftColumnIndex; } while (sourceSelection.bottomRowIndex > sourceSelection.topRowIndex++); } break; case ... } };
Добавлено много граблей в коде копирования значений между массивами.
function func1(i) { return i++; }
Многие отвечают 11 и это сразу показывает, что тут есть грабли: автор написал такой код, на чтении которого происходит неверное понимание логики работы этого кода. Автор тут сделал серьезную ошибку в своей работе: создал новый источник расходов на сопровождение написанного кода.
В долгих проектах код пишется один раз, а читается имхо ~1000 раз и при оценке нового кода нужно смотреть на ошибки понимания логики работы этого кода при беглом чтении исходников.
Так же нет смысла упаковывать код в минимальное количество строк: это требует внимательного прочтения каждого выражения в каждой строке и мысленного построения всей последовательности вычислений всех выходных значений (с подходом "в одной строке" этих значений часто бывает несколько и вся конструкция может быть довольно сложной).
Нужно сделать понимание логики работы кода с первого раза при беглом чтении даже если что-то из языка подзабылось. Минимальное количество строк тут часто мешает и к нему точно не нужно стремиться.
Так же очень легко неправильно вспомнить (вообще не вспомнить) приоритеты выполнения операций или ошибиться с логикой вычисления конечных значений.
Например, как это все относится к одной строке кода из моего фрагмента:
do s.SetCellValue(destSelection.leftColumnIndex + (i++), destSelection.topRowIndex, s.getCellValue(sourceSelection.leftColumnIndex, sourceSelection.topRowIndex) ?? '');
На первом заходе i=0 и надо не просто прочитать "запись в ячейку destSelection.leftColumnIndex + 1", а дополнительно вспомнить про разные приоритеты у "+" и "++": "будет в ячейку destSelection.leftColumnIndex + значение (i + 1) потому что "++" выполняется сразу" или все таки сделан алгоритм "прибавляется текущее значение i, а уже после вызова setCellValue будет изменение i на +1"?
Такой код лучше сразу переделать на вариант, когда не требуется вспоминать приоритеты операций между собой и даже вообще знать про наличие приоритетов между этими операциями.
То же самое для другой строки из моего фрагмента:
while (sourceSelection.bottomRowIndex > sourceSelection.topRowIndex++);Нужно не просто прочитать "rightColumnIndex больше (leftColumnIndex+1)", а дополнительно вспомнить приоритет между логической операцией ">" и операцией "++" и оказывается тут условие "rightColumnIndex больше leftColumnIndex", а уже после сравнения будет увеличение значения leftColumnIndex на +1
Такой код лучше сразу переделать на вариант, когда это вспоминать вообще не требуется.
Вторые популярные грабли - это несколько операций в одной строке.
Например, эта строка:
destSelection.topRowIndex++; sourceSelection.leftColumnIndex = selection.leftColumnIndex;
В одной строке сделаны операции увеличения значения в свойстве одного объекта и изменение свойства другого объекта. При беглом чтении второе выражение очень легко пропустить и сложится неверное понимание работы всего алгоритма.
Или другая строка:
do s.SetCellValue(destSelection.leftColumnIndex + (i++), destSelection.topRowIndex, s.getCellValue(sourceSelection.leftColumnIndex, sourceSelection.topRowIndex) ?? '');
Тут строка сразу не влезает в экран. Надо внимательно изучить ВСЮ строку до последних символов что бы понять какое именно значение будет присвоено в ячейку: где начинается это получение нужного значения, где оно заканчивается и какие ветвления есть в этом получении значения.
В одной строке сделаны операции цикла, изменения значения ячейки, вычисления индекса текущей ячейки, перевод индексной переменной на новую ячейку, получение значения из другой ячейки, варианты ветвления для выбора значения.
Третьи популярные грабли - это разные алгоритмы для одинаковых задач
Например, в этой строке реализованы разные алгоритмы для одной и той же задачи:
s.SetCellValue(destSelection.leftColumnIndex + (i++), destSelection.topRowIndex, s.getCellValue(sourceSelection.leftColumnIndex, sourceSelection.topRowIndex) ?? '');Адресация в destSelection сделана через неизменное значение leftColumnIndex и новую переменную для сдвига на следующую ячейку (ее значение увеличивается на +1 на каждом цикле).
Адресация в sourceSelection в этой строке выглядит как "значение всегда берется из одной и той же ячейки, ведь для последовательного прохода по всем ячейкам автор уже выбрал алгоритм "left+i", а для sourceSelection в коде использован только leftColumnIndex рядом с которым нет "+ i".
Что бы правильно понять логику работы этого кода нужно внимательно прочитать другие строки в этом методе. Сдвиг на следующий столбец для sourceSelection.leftColumnIndex реализован через другой алгоритм ("изменяемое значение leftColumnIndex") и даже немного припрятан:
while (sourceSelection.rightColumnIndex > sourceSelection.leftColumnIndex++);Отрефакторить или написать с нуля?
Я бы написал заново: алгоритм довольно небольшой и тесты есть на результат работы всего алгоритма.
Например тут можно использовать 'for' с явно отделенными друг от друга логическими операциями и операцией ++ (и не надо даже знать про существование приоритетов между этими операциями), с отделенными операциями сложения и операцией ++ (не надо помнить их приоритеты), с выносом получения значения на отдельную строку (нет граблей с сложностями чтения длинных строк с несколькими операциями в одной строке), с одинаковой реализацией алгоритма адресации в обоих массивах (нет граблей с пониманием "откуда берется значение"):
const colCount = sourceSelection.rightColumnIndex - sourceSelection.leftColumnIndex + 1; const rowCount = sourceSelection.topRowIndex - sourceSelection.bottomRowIndex + 1; for (let rowShift = 0; rowShift < rowCount; rowShift++) { for (let colShift = 0; colShift < colCount; colShift++) { const sourceValue = s.getCellValue(sourceSelection.leftColumnIndex + colShift, sourceSelection.topRowIndex + rowShift) ?? ''; s.SetCellValue(destSelection.leftColumnIndex + colShift, sourceSelection.topRowIndex + rowShift, sourceValue); } }
Больше примеров похожего кода есть в https://learn.javascript.ru/ninja-code
Комментариев нет:
Отправить комментарий