7 способов улучшить код

Здравствуй, брат автоматизатор!

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

Небольшое отступление:
Полгода назад я задумал написать серию статей о программировании для новичков, где я писал код и объяснял, почему он должен писаться так, а не иначе. Но такое объяснение оказалось для меня непосильным занятием, т. к. мне приходилось спорить с самим собой. Легко было только один раз, когда я обнаружил серьезный дефект, наверное, во всех копалках на проекте. Но потом я получил стрелу в колено...
Вернувшись на проект, я неожиданно для себя понял, что демонстрировать эффективность удачного кода значительно проще, противопоставляя его коду неудачному.

В прошлый раз я рассказал о том, как математика помогает писать более простые, понятные и при этом более эффективные программы.
qwertyMAN заметил, что использование взятия остатка от деления – это не математическая проблема, и я с ним отчасти согласился. Решение о том, использовать или нет операцию %, мало влияет на общий алгоритм, но сильно влияет на то, как будет выглядеть готовый код. Но всё-таки об этой операции следует помнить уже на этапе проектирования алгоритма, а не только на этапе кодинга.
А вот, о тонкостях написания кода я хочу рассказать сегодня, не особо вдаваясь в общие алгоритмы. Моим подопытным будет уже знакомый код из охранной системы турелей.
Вот сам код:

local function func()
	local com					= require("component")
	local event					= require("event")
	local gpu					= com.gpu
	local turret				= com.os_energyturret
	local radar					= com.openperipheral_sensor
	local autors				= {"qwertyMAN"}
	local version				= "0.9.1"

	-- Настройки
	local firePleayers			= true
	local fireMobs				= true
	local Black_Player_List		= {}
	local White_Player_List		= {}
	-- относительные координаты пушки от сканера
	local correct = {
		x = 0,
		y = 4,
		z = 0
	}

	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		
					
	while true do
		os.sleep(0)
		local target = false
		local fire = true
		local scan=radar.getPlayers()
		if firePleayers and #scan>0 then
			if #White_Player_List>0 then
				for i=1, #autors do
					White_Player_List[#White_Player_List+1] = autors[i]
				end
				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
						end
					end
					if swich then
						target = scan[i].name
						break
					end
				end
			elseif #Black_Player_List>0 then
				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 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
						end
					end
					if swich then
						target = scan[i].name
						break
					end
				end
			else
				if #autors>0 then
					for i=1, #autors do
						White_Player_List[#White_Player_List+1] = autors[i]
					end
				else
					target = scan[1].name
				end
			end
			if target and radar.getPlayerByName(target) then
				target=radar.getPlayerByName(target).all()
				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 vx,vy = pointer(x,y,z)
				turret.moveTo(vx,vy)
				if turret.isOnTarget() then
					turret.fire()
				end
				fire = false
			end
		end
		target = false
		if fireMobs and fire then
			local scan=radar.getMobIds()
			if #scan>0 then
				for i=1, #scan do
					local mob
					if radar.getMobData(scan[i]).basic() then
						mob = radar.getMobData(scan[i]).basic()
						target = mob
					end
				end
				if target then
					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 vx,vy = pointer(x,y,z)
					turret.moveTo(vx,vy)
					if turret.isOnTarget() then
						turret.fire()
					end
				end
			end
		end
	end
end
-- мегакостыль
while true do
	local oop = pcall(func)
	print(oop)
end

Первое, на что я обратил внимание – это использование 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

if target then
  target = radar.getPlayerByName(target)
  if target then
    target=target.all()
...
target = radar.getMobData(scan[i])
if target then
  target = target.basic()

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

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()

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 = первый игрок на радаре
  вычисление координат цели и выполнение выстрела
  сканирование мобов, вычисление их координат с последующим расстрелом.

В программе присутствуют и менее очевидные вычисления, которые можно вынести за цикл.

-- этот код находится за циклом
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

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

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

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"])

Как это соотносится с нашим кодом? Попробуем заполнять списки игроков таким образом:
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
end

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

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'а вполне адекватен.

Программируй красиво, брат!