






7 способов улучшить код
Здравствуй, брат автоматизатор!
Желаешь ли ты услышать от меня несколько мыслей о красивом, понятном, и эффективном коде?
На истину в последней инстанции я не претендую, и ты, возможно, знаешь решения получше моих. В этом случае оставь свои соображения в комментариях, и код на проекте станет немного чище.
Небольшое отступление:
Полгода назад я задумал написать серию статей о программировании для новичков, где я писал код и объяснял, почему он должен писаться так, а не иначе. Но такое объяснение оказалось для меня непосильным занятием, т. к. мне приходилось спорить с самим собой. Легко было только один раз, когда я обнаружил серьезный дефект, наверное, во всех копалках на проекте. Но потом я получил стрелу в колено...
Вернувшись на проект, я неожиданно для себя понял, что демонстрировать эффективность удачного кода значительно проще, противопоставляя его коду неудачному.
В прошлый раз я рассказал о том, как математика помогает писать более простые, понятные и при этом более эффективные программы.
qwertyMAN заметил, что использование взятия остатка от деления – это не математическая проблема, и я с ним отчасти согласился. Решение о том, использовать или нет операцию %, мало влияет на общий алгоритм, но сильно влияет на то, как будет выглядеть готовый код. Но всё-таки об этой операции следует помнить уже на этапе проектирования алгоритма, а не только на этапе кодинга.
А вот, о тонкостях написания кода я хочу рассказать сегодня, не особо вдаваясь в общие алгоритмы. Моим подопытным будет уже знакомый код из охранной системы турелей.
Вот сам код:
Первое, на что я обратил внимание – это использование pcall. Именно оно и побудило меня к разбору кода, но прежде я предлагаю обсудить другой нюанс.
1. Избегай повторения уже выполненных вычислений
Имеются два фрагмента кода:
if target and radar.getPlayerByName(target) then target=radar.getPlayerByName(target).all() ... local mob if radar.getMobData(scan[i]).basic() then mob = radar.getMobData(scan[i]).basic() target = mobИ в обоих фрагментах происходит дублирование вызовов. Первый раз вроде как для проверки. Но такой код всё равно не снимает проблему, т. к. при первом вызове функция может вернуть одно значение, а при втором вернет другое. Правильным решением будет сохранить результат в переменную. И если ее значение удовлетворяет условиям, использовать ее в дальнейшем. Во втором фрагменте следует заодно избавиться от переменной mob, которая больше нигде не используется.
if target then target = radar.getPlayerByName(target) if target then target=target.all() ... target = radar.getMobData(scan[i]) if target then target = target.basic()Почему я проверяю результаты getPlayerByName и getMobData, а не других функций вроде all() или basic()? Потому что именно они могут вызвать ошибку. Ошибка возникает от того, что игрок или моб может покинуть зону действия сенсора, пока выполняется обработка внутри программы. К сожалению, указанной проверки недостаточно, т. к. мод OpenPeripheral вместо того чтобы вернуть nil или иным образом указать на проблему, тупо генерирует ошибку, не оставляя иного варианта кроме использования pcall.
2. Используй защищенный режим только там, где это необходимо
Код программы выглядит примерно так:
local function func() -- почти весь код программы помещен в эту функцию -- включая инициализацию переменных и определение функций ... -- этот бесконечный цикл прерывается из-за отсутствующей обработки ошибок while true do ... local target = false local scan=radar.getPlayers() ... target = ... if target and radar.getPlayerByName(target) then target=radar.getPlayerByName(target).all() ... end ... end end -- мегакостыль (комментарий самого автора) while true do local oop = pcall(func) print(oop) endАвтор понимает, что использует костыль, но правильное решение использовать не хочет. Подобные городушки провоцируют появление новых: уже сейчас на ровном месте появилась дополнительная функция и вложение двух бесконечных циклов. Но главная проблема этого кода в том, что pcall скрывает любые ошибки, и дальнейшая отладка программы становится затруднительной. Даже печать результата, возвращаемого pcall, реализована неверно – ничего кроме false выведено не будет.
Для решения проблемы следует использовать pcall точечно, только там, где это необходимо, а именно в вызове getPlayerByName и getMobData. А если есть возможность вообще обойтись без pcall, то в готовой программе без него следует обойтись. В нашем случае обойтись без pcall, похоже, нельзя. Поэтому убираем наш костыль и функцию func, оставив лишь ее содержимое, и дорабатываем проблемные участки таким образом:
if target then local flag,res = pcall(radar.getPlayerByName,target) if flag then target=res.all() ... local flag,res = pcall(radar.getMobData,scan[i]) if flag then target = res.basic()Пришлось ввести дополнительные переменные, зато код избавился от костыля со всей его обвязкой, а ошибка, генерируемая модом OpenPeripheral, локализована, и новых проблем не создаёт. Программа работает стабильно и ее отладка не нарушена. На этом можно было бы и закончить, но я уже вошёл во вкус. Поэтому продолжу давать советы, иллюстрируя их фрагментами кода:
3. Выноси за цикл всё, что возможно
Это правило подобно первому, но пренебрежение им не так бросается в глаза начинающего программиста, т.к. сам код не дублируется, зато дублируется его исполнение.
Восстановим общий алгоритм работы программы, чуть подробнее разобрав этот кусок:
while true do local target = false ... local scan=radar.getPlayers() if firePleayers and #scan>0 then if #White_Player_List>0 then добавление списка авторов в в белый список target = первый игрок на радаре вне белого списка elseif #Black_Player_List>0 then удаление авторов из черного списка target = первый игрок на радаре в черном списке else if #autors>0 then перенос авторов в белый список else target = первый игрок на радаре вычисление координат цели и выполнение выстрела сканирование мобов, вычисление их координат с последующим расстрелом.Как ты уже догадался по заголовку, в бесконечном цикле выполняется что-то явно лишнее. А именно, работа со списком авторов. Всё это могло бы прекрасно работать и до основного цикла, создавая при этом нужный эффект. Более того, вынос этого кода за цикл позволит исправить серьезнейшую ошибку: на каждой итерации бесконечного цикла происходит добавление авторов в белый список, при этом их наличие в белом списке никак не проверяется, но каждый раз происходит добавление новых. Даже с двумя планками памяти 3.5 уровня через сотню-другую тысяч итераций программа завершится с ошибкой «not enough memory».
Вынося лишние действия за цикл, ты избавишь программу как от неконтролируемого расхода памяти, так и от лишних действий, замедляющих ее работу. Думаю, демонстрировать корректный код здесь излишне. Достаточно лишь вынести эти участки кода из цикла.
В программе присутствуют и менее очевидные вычисления, которые можно вынести за цикл.
-- этот код находится за циклом local correct = { x = 0, y = 4, z = 0 } ... -- а этот внутри local x,y,z = target.position.x-0.5-correct.x, target.position.y+0.3-correct.y, target.position.z-0.5-correct.z ... local x,y,z = target.position.x-0.5-correct.x, target.position.y-0.5-correct.y, target.position.z-0.5-correct.zЛогичным будет переписать код таким образом:
-- этот код за циклом local correct = { x = 0+0.5, y = 4+1, z = 0+0.5 } ... -- а этот внутри local x,y,z = target.position.x-correct.x, target.position.y+1.3-correct.y, target.position.z-correct.z ... local x,y,z = target.position.x-correct.x, target.position.y+0.5-correct.y, target.position.z-correct.zТак мы избавимся от лишних сложений и вычитаний в цикле. Лишние сложения при инициализации таблицы correct не сильно помещают, т. к. они выполняются всего один раз, а нужны они для наглядности кода. Поясню, что здесь происходит.
Во-первых, турель и сенсор находятся в разных блоках, поэтому координаты игрока следует скорректировать на эту разницу. Координаты турели относительно сенсора хранятся в таблице correct.
Во-вторых, сенсор определяет координаты игроков и мобов относительно своего северо-западного угла, а турель вращается вокруг центра блока. Поэтому приходится корректировать координаты (x,z) на половину блока по горизонтали.
В-третьих, сама турель стреляет на один блок выше уровня ног игрока или моба. Автор корректирует высоту цели в зависимости от цели: для игрока прицел приподнимается на 0.3 блока, чтобы игрок не мог уходить от выстрела прыжком или прятаться за блоки, а для мобов прицел опускается на полблока, чтобы попадать, например, в кур или свиней. Но тут тоже не всё просто. Чтобы попадать в цыплят, прицел следует опустить еще ниже, но тогда турель промахивается мимо взрослых кур, стреляя им куда-то под ноги. Для эффективной стрельбы по любым мобам нужен алгоритм, определяющий вид моба, его возраст, и находящий по таблице его рост. Причем, нужен не только рост. Например, нет смысла целиться в нижнюю часть слизня, т. к. тот постоянно прыгает. В общем, это отдельная тема для поиска оптимального алгоритма, сейчас же я хочу продолжить рассказ о кодинге.
В итоговом коде не только вынесены лишние вычисления из цикла. Кроме этого он стал более логичным: числа 1.3 и 0.5 по сути означают высоту цели относительно ног игрока или моба.
4. Прерывай циклы, когда они уже выполнили свою задачу
Вот два подобных друг другу фрагмента кода:
local swich = true for j=1, #White_Player_List do if scan[i].name==White_Player_List[j] or not scan[i].name then swich = false end end if swich then ... local swich = false for j=1, #Black_Player_List do if scan[i].name==Black_Player_List[j] then swich = true end end if swich thenЗадача циклов в том, чтобы при подходящем случае изменить значение переменной switch. Но как только оно изменилось, зачем продолжать работу? Далай break. Иначе выполнение твоей программы замедляется.
Пока я писал этот текст, то напрягался при каждом наборе названия переменной swich. Нет такого слова, зато есть слово switch, и не глядя я набирал именно его. Поэтому буду писать switch. Кроме того, название переменной все равно не отражает ее сути. С тем же успехом можно было использовать однобуквенное название переменной. А лучше бы и вовсе избавиться от нее.
5. Избавляйся от лишних переменных
Вот те же фрагменты в немного дополненном составе, внутри других циклов и с оператором break, как же теперь без него:
local target = false ... for i=1, #scan do local swich = true for j=1, #White_Player_List do if scan[i].name==White_Player_List[j] or not scan[i].name then swich = false break end end if swich then target = scan[i].name break end end ... for i=1, #scan do local swich = false for j=1, #Black_Player_List do if scan[i].name==Black_Player_List[j] then swich = true break end end if swich then target = scan[i].name break end endЧто выполняют оба фрагмента? Ищут подходящую цель по белому и черному спискам игроков.
Где хранится цель? В переменной target.
Что хранится в переменной switch? Флаг того, что переменная target должна быть изменена.
А зачем нам этот флаг? Что мешает сразу изменить переменную?
local target = false ... for i=1, #scan do target = scan[i].name for j=1, #White_Player_List do if scan[i].name==White_Player_List[j] or not scan[i].name then target = false break end end if target then break end end ... for i=1, #scan do for j=1, #Black_Player_List do if scan[i].name==Black_Player_List[j] then target = scan[i].name break end end if target then break end endКод стал немного короче и быстрее, но и это еще не предел.
Еще непонятно, что делает, or not scan[i].name в условии. Повлиять это выражение может в том случае, если scan[i].name будет равно false, или nil. А разве такое возможно? Даже не знаю, как классифицировать этот недочет. Видимо, от старых экспериментов осталось. Посоветовать можно только одно: вычищать код перед публикацией.
6. Используй ассоциативные возможности таблиц Lua
Таблицы в Lua – это больше чем массивы. Это и объекты, и списки, и словари. А может, и что-то еще, о чем я забыл или даже не знал.
Таблицы в Lua – это наборы пар ключ-значение. В качестве ключей и значений таблицы может быть что угодно кроме nil.
Пара ключ-значение в таблицах Lua присутствует даже если мы не используем ключи явным образом.
Попробуй запустить такой код:
PlayerList={"Ded", "Baba", "KurochkaRyaba"} for k,v in pairs(PlayerList)do print(k,v)end for i=1,#PlayerList do print(PlayerList[i])end PlayerList={[1]="Ded", [2]="Baba", [3]="KurochkaRyaba"} for k,v in pairs(PlayerList)do print(k,v)end for i=1,#PlayerList do print(PlayerList[i])end PlayerList={["Ded"]=1, ["Baba"]=2, ["KurochkaRyaba"]=3} for k,v in pairs(PlayerList)do print(k,v)end for i=1,#PlayerList do print(PlayerList[i])end print(#PlayerList) print(PlayerList["Ded"]) print(PlayerList["Baba"]) print(PlayerList["RedHatBaby"])

В первом случае мы не указываем ключи явно, но они создаются. Мы свободно перебираем как пары ключ-значение, так и значения по их индексу, который совпадает с ключом.
Во втором случае мы явно указали ключи. Перебор пар ключ-значение показывает, что элементы таблицы хранятся в неведомом нам порядке, скорее всего, в соответствии с неким хешем, но на перебор по индексу это никак не влияет, порядок не нарушен, а это самое главное.
В третьем случае мы поменяли ключи и значения местами. Естественно, ни о каком переборе по индексу теперь не идет и речи. Более того, определить длину таблицы теперь тоже не так просто. Зато появилась возможность, не выполняя перебор всех значений в цикле, одной командой определить, присутствует ли игрок в списке. Для игроков Дед и Баба есть значение, а игрок КраснаяШапочка в список не внесен, и имеет значение nil.
Как это соотносится с нашим кодом? Попробуем заполнять списки игроков таким образом:
local Black_Player_List = { ["qwertyMAN"]=1 }
local White_Player_List = { ["Ded"]=1, ["Baba"]=1, ["KurochkaRyaba"]=1 }
В качестве значения я использовал 1, и оно может быть любым, но 1 - записывается кратко. Главное, чтобы не nil. И не false, чтобы проще было выполнять проверку элемента.
То, что qwertyMAN оказался в черном списке, ему никак не повредит, он же автор.
Теперь все фрагменты кода изменятся таким образом:
Добавление списка авторов в в белый список:
-- было for i=1, #autors do White_Player_List[#White_Player_List+1] = autors[i] end -- стало for i=1, #autors do White_Player_List[autors[i]] = 1 endКод немного укоротился, а главное – теперь переполнение памяти не грозит даже при работе в бесконечном цикле, т. к. элемент создается один раз, а в последующие – лишь изменяется его значение.
Удаление авторов из черного списка:
-- было for i=#Black_Player_List, 1, -1 do for j=1, #autors do if Black_Player_List[i] == autors[j] then table.remove(Black_Player_List,i) end end end -- стало for j=1, #autors do Black_Player_List[autors[i]] = nil endКод заметно укоротился.
Сомневаешься, действительно ли элемент таблицы удаляется? Запусти этот код, и все станет понятным:
PlayerList={["Ded"]=1, ["Baba"]=2, ["KurochkaRyaba"]=3} for k,v in pairs(PlayerList)do print(k,v)end PlayerList["Ded"]=nil for k,v in pairs(PlayerList)do print(k,v)endИ напоследок два уже разобранных перед этим фрагмента, которые еще более упростились:
-- было for i=1, #scan do target = scan[i].name for j=1, #White_Player_List do if scan[i].name==White_Player_List[j] then target = false break end end if target then break end end ... for i=1, #scan do for j=1, #Black_Player_List do if scan[i].name==Black_Player_List[j] then target = scan[i].name break end end if target then break end end -- стало for i=1, #scan do if not White_Player_List[scan[i].name] then target = scan[i].name break end end ... for i=1, #scan do if Black_Player_List[scan[i].name] then target = scan[i].name break end end7. Минимизируй идентичные участки кода
При добавлении сходного функционала в программу можно скопировать рабочий участок кода и немного изменить его. Иногда до неузнаваемости. Но если фрагмент достаточно велик, а изменения малы, то правильнее будет вынести этот фрагмент в отдельную функцию. Во-первых, итоговой код будет более компактным. Во-вторых, при необходимости будет проще вносить изменения, не правя все участки кода.
В программе имеется два таких похожих участка:
local x,y,z = target.position.x-correct.x, target.position.y+1.3-correct.y, target.position.z-correct.z local vx,vy = pointer(x,y,z) turret.moveTo(vx,vy) if turret.isOnTarget() then turret.fire() end ... local x,y,z = target.position.x-correct.x, target.position.y+0.5-correct.y, target.position.z-correct.z local vx,vy = pointer(x,y,z) turret.moveTo(vx,vy) if turret.isOnTarget() then turret.fire() endРанее мы уже выяснили, что единственная изменяемая величина здесь – это коррекция высоты цели. Сейчас основной вопрос: что из этого следует вынести в отдельную функцию. С точки зрения минимизации кода следует выносить почти всё. Но чтобы сделать код более логичным, не следует всё мешать в одну кучу. Лучшим решением мне кажется вынос всего, что связано с вычислениями, в уже имеющуюся функцию pointer. Вот она:
local function pointer(x,y,z) local distXY = math.sqrt(x^2+z^2) local distDY = math.sqrt(y^2+distXY^2) local outX = math.deg(math.acos(x/distXY))+90 local outY = 90-math.deg(math.acos(y/distDY)) if z<0 then outX = (180-outX)%360 end return outX,outY endУчитывая то, как я переписал эту функцию в прошлый раз, а также избавляясь от лишних переменных и перенося часть вычислений внутрь функции, перепишу код таким образом:
local function pointer(pos,h) local x,y,z = pos.x-correct.x, pos.y-correct.y+h, pos.z-correct.z local azimuth=math.deg(math.atan2(x,-z))%360 local elevation=math.deg(math.atan2(y,math.sqrt(x*x+z*z))) return azimuth, elevation end ... turret.moveTo(pointer(target.position,1.3)) if turret.isOnTarget() then turret.fire() end ... turret.moveTo(pointer(target.position,0.5)) if turret.isOnTarget() then turret.fire() endКонечно, этот код можно ужать еще плотнее, но я увижу в этом смысл, если дальнейшая доработка программы вынудит меня продублировать эти блоки кода.
На этом закончу. Возможно, я дал бы тебе еще пару советов, но за исключением описанных выше моментов код qwertyMAN'а вполне адекватен.
Программируй красиво, брат!
- Zer0Galaxy, Krutoy, Fingercomp и 4 другим это нравится
TL:DR
Но в роде норм. Так что лайк.