Code optimization suggestions request! - ComputerCraft Forums (archive)

JessiahC #1

10 posts

Posted 04 December 2013 - 01:33 AM

First of all, let me thank you for taking the time to read through this. I am a LUA noob, and so far have learned most of what I know through the lua.org website and through this website as well. I appreciate what all of you guys do for the ComputerCraft community.

I want to start off by talking about this program and letting you know that it does work, so we're off to a good start. Whether it works well is subjective to one's knowledge of LUA, hence why I'm posting here. I feel as though what I'm trying to accomplish could be done better if I knew more or if I knew the right syntax to get my point across. I don't have a bunch of comments in my code because I hope I wrote it for the most part as readable (Good code is it's own documentation they say). If not, I can easily break it down into chunks that provide details explaining what each bit does. I'm using CC version 1.57 and OP 0.2.1, but our server is due for mod updates soon.

There are a lot of "~= nil" conditionals in there, because unavoidably I receive nil statements that I haven't quiet ironed out of yet, but I've got most of the bugs squashed and it's been running well for a couple days now.

What I'm looking for here is constructive criticism, and comments like "Hey, why'd you do that when you could have just done this much simpler thing…." Most of the time my answer will be "Holy crap you can do that?!" Any advice you could offer to make this code more streamline/efficient would be greatly appreciated. Again, thank you for taking the time to wade through my programming.

Here are the goals for the program:
1. Pull the stored energy data from the 9 network peripheral attached MFSUs
2. Calculate the total energy stored and total energy percentage and write it
3. Calculate the stored energy data into percentages, color them by percentage, write them on the monitor
4. Draw energy flow chart statics (box and scale text)
5. Find the difference in total energy values for 1 second, divide it down into segments and chart them
6. Have the chart scroll to the left, per second, to monitor changes in energy flow.

This is a animated .jpeg of the working program running: (Alt Link if not displayed below)

I tried to cut it down to 200 lines. I was not going to CODE it here because it was slightly over, but I figured it might save you a bit of trouble. It's not like a 500+ line OS or anything crazy like that. Here's the link to the pastebin, and the code is below.


mfsu = {}
mfsu["1"] = peripheral.wrap("mfsu_3")
mfsu["2"] = peripheral.wrap("mfsu_4")
mfsu["3"] = peripheral.wrap("mfsu_5")
mfsu["4"] = peripheral.wrap("mfsu_6")
mfsu["5"] = peripheral.wrap("mfsu_7")
mfsu["6"] = peripheral.wrap("mfsu_8")
mfsu["7"] = peripheral.wrap("mfsu_9")
mfsu["8"] = peripheral.wrap("mfsu_10")
mfsu["9"] = peripheral.wrap("mfsu_11")
m = peripheral.wrap("monitor_13")
m.clear()
storedEU = {}
storedPerc = {}
plotter = {}
printer = {}
energyTotals = 0
energyFlow = 0

function storedTotal()
energyTotal = 0
for i = 1,9 do
   energyTotal = energyTotal + mfsu[tostring(i)].getEUStored()
end
energyFlow = (energyTotal - energyTotals) / 20
energyTotals = energyTotal
energyPercentage = shorten(tostring(energyTotal / 360000000 * 100))
m.setCursorPos(3,3)
m.write("Total Energy Stored: "..energyTotal.." EU")
m.setCursorPos(9,4)
m.write("Total Capacity: ")
colorPerc(energyPercentage)
end

function storedEach()
for i = 1,9 do
   storedEU[i] = mfsu[tostring(i)].getEUStored()
   storedPerc[i] = shorten(tostring(storedEU[i] / 40000000 * 100))
end
m.setCursorPos(1,6)
for i = 1,3 do
  m.write("MFSU"..i..": ")
  colorPerc(storedPerc[i])
  xPos = i * 14
  m.setCursorPos(xPos,6)
end
m.setCursorPos(1,7)
for i = 4,6 do
  m.write("MFSU"..i..": ")
  colorPerc(storedPerc[i])
  xPos = (i-3) * 14
  m.setCursorPos(xPos,7)
end
m.setCursorPos(1,8)
for i = 7,9 do
  m.write("MFSU"..i..": ")
  colorPerc(storedPerc[i])
  xPos = (i-6) * 14
  m.setCursorPos(xPos,8)
end
end

function shorten(num)
if num ~= nil then
  dec = string.find(tostring(num),".",1,true)
  if dec ~= nil then
   dec = dec + 1
   return (string.sub(num,1,dec))
  end
  else
return ("0")
end
end

function percRound(num)
subW,subR,whole,remainder = 0,0,0,0
subW,subR = string.len(num) - 2 , string.len(num)
whole = tonumber(string.sub(num,1,subW))
remainder = tonumber(string.sub(num,subR,subR))
if remainder >= 5 then
  whole = whole + 1
  end
return whole
end

function colorPerc(num)
num = tonumber(num)
if num ~= nil then
  if num >= 0 then
	m.setTextColor(colors.red)
  end
  if num > 30 then
	m.setTextColor(colors.orange)
  end
  if num > 60 then
	m.setTextColor(colors.yellow)
  end
  if num > 90 then
	m.setTextColor(colors.lime)
  end
   m.write(num.."%")
   space = string.len(tostring(num)) - 5 * (-1)
  for i = 1,space do
   m.write(" ")
  end
  m.setTextColor(colors.white)
end
end

function flowChart()
for i = 1,15 do
  m.setCursorPos(6,(i + 10))
  m.write("|")
end
for i = 1,39 do
  m.setCursorPos(i,26)
  m.write("-")
end
for i = 1,15 do
  m.setCursorPos(34,(i + 10))
  m.write("|")
end
for i = 1,2 do
  if i == 1 then
	yP = 1
	else
	yP = 35
  end
  m.setCursorPos(yP,11)
  m.write("2.1k ")
  m.setCursorPos(yP,13)
  m.write("1.5k ")
  m.setCursorPos(yP,15)
  m.write(" 900 ")
  m.setCursorPos(yP,17)
  m.write(" 300 ")
  m.setCursorPos(yP,19)
  m.write("-300 ")
  m.setCursorPos(yP,21)
  m.write("-900 ")
  m.setCursorPos(yP,23)
  m.write("-1.5k")
  m.setCursorPos(yP,25)
  m.write("-2.1k")
end
end

function drawPlots()
m.setCursorPos(12,26)
m.write("--("..energyFlow.." EU/t)--------")
yPos = plotPos(energyFlow) + 18
printer[34] = yPos
for i = 7,33 do
  j = i + 1
  h = i
  printer[i] = printer[j]
  for i = 11,25 do
   m.setCursorPos(h,i)
   if i == printer[h] then
	if i < 18 then
	 m.setBackgroundColor(colors.cyan)
	 else
	 m.setBackgroundColor(colors.blue)
	end
   else
	m.setBackgroundColor(colors.black)
   end
   m.write(" ")
   m.setBackgroundColor(colors.black)
  end
end
end

function plotPos(num)
plotter,plotW,plotR = 0,0,0
plotter = energyFlow / 300
if plotter < 0 then
  plotW = tonumber(string.sub(tostring(plotter),2,2))
  plotR = tonumber(string.sub(tostring(plotter),4,4))
  negative = true
else
  plotW = tonumber(string.sub(tostring(plotter),1,1))
  plotR = tonumber(string.sub(tostring(plotter),3,3))
  negative = false
end
if plotR ~= nil then
  if plotR >= 5 then
   plotW = plotW + 1
  end
end
if negative == false then
  plotW = plotW * (-1)
end
return plotW
end

function label()
m.setCursorPos(13,1)
m.write("Energy Monitor")
m.setCursorPos(1,10)
m.write("---------- Energy Flow (EU/t)----------")
end

label()
flowChart()

while true do
storedTotal()
storedEach()
drawPlots()
sleep(1)
end

If it's too confusing or there's something that doesn't make sense to you (which is likely to happen since I'm still a LUA white belt), please quote out a section of code or just say you can't understand any of it, hah, and I can explain what it's "supposed" to do line by line. I did my best to make it easy to read. I appreciate all the help and support.

theoriginalbit #2

7508 posts

Location Australia

Posted 04 December 2013 - 01:49 AM

There's definitely a few things that you can do to improve this. The first being the localisation of functions and variables. Local variables/functions are not only quicker to access but once the program has finished running they do not remain as a pollutant in the environment. You may have to reorder some of your functions once localised so that you can invoke them.

Other things include fixing your indentation, the convention is at the start of each code block the indentation level increases, then it decreases again once the end is reached. Your main problems with it are the functions.

You do have a few redundant calls in your code and your "shorten" function, is an interesting method to achieving rounding. You could have used string.format("%.1f", num) there to do the same thing.

there's just a few other small things like your checks for == false and == nil etc… In Lua false or nil both resolve to false in conditionals, and any other value will resolve to true.

Its a nice short program so I'll do a little bit of a refactor for you and post it when done.

EDIT: Oh quick question, are the MFSU's the only ones connected to the network? or are the specific ones?

Edited on 04 December 2013 - 12:50 AM

JessiahC #3

10 posts

Posted 04 December 2013 - 02:03 AM

You're definitely on point with those suggestions, thank you! That computer and the MFSUs are on their own network, so it's only those 9.

theoriginalbit #4

7508 posts

Location Australia

Posted 04 December 2013 - 02:38 AM

Just a bunch of little changes here and there. mostly all commented. http://pastebin.com/6b2UChCG

Some areas that I'd improve if I were doing it myself, hopefully you can see where they'd go 'cause its not the full program. http://pastebin.com/XnWxXvXF

Then there's some things you can do just generally in plotPos.

here's the fix


local function plotPos() --# you never used "num"
  local plotter,plotW,plotR = energyFlow/300,0,0
  if plotter < 0 then
	--# I don't actually know the purpose of this, I have a feeling it can be much better though
	plotW = tonumber(string.sub(tostring(plotter),2,2))
	plotR = tonumber(string.sub(tostring(plotter),4,4))
  else
	plotW = tonumber(string.sub(tostring(plotter),1,1))
	plotR = tonumber(string.sub(tostring(plotter),3,3))
  end

  if plotR and plotR >= 5 then
	plotW = plotW + 1
  end

  return math.abs(plotW) --# return the absolute, or non-negative value
end

Edited on 04 December 2013 - 01:40 AM

JessiahC #5

10 posts

Posted 04 December 2013 - 10:43 AM


local function plotPos() --# you never used "num" , --It sources it's data for the function from the energyFlow variable a couple functions up.
  local plotter,plotW,plotR = energyFlow/300,0,0

		--#Here's what this does. When you divide down energyFlow, it'll come down to a number with a decimal most cases, i.e. 3.743 or -1.2

  if plotter < 0 then   -if the energyFlow is a negative value
		plotW = tonumber(string.sub(tostring(plotter),2,2)) --#Turn the variable into a string, extract the second character in the string, and turn it back into a number so -1.2 becomes just (1)
		plotR = tonumber(string.sub(tostring(plotter),4,4))  --#Turn the variable into a string, extract the fourth character in the string, and turn it back into a number so -1.2 becomes just (2)
  else
		plotW = tonumber(string.sub(tostring(plotter),1,1))  --#Same as above, but since it's not a negative number, it won't try to exclude the negative symbol
		plotR = tonumber(string.sub(tostring(plotter),3,3))
  end
  if plotR and plotR >= 5 then  --#This rounds the remainder up
		plotW = plotW + 1		   -- #and added it to the whole number
  end
  ---#at this point, regardless of whether the energyFlow came in positive or negative, it will always be positive (not what I want), so to ensure I get the negative back, I added the negative == true variable
  return math.abs(plotW) --# return the absolute, or non-negative value
end

Edited on 04 December 2013 - 09:44 AM

theoriginalbit #6

7508 posts

Location Australia

Posted 04 December 2013 - 06:19 PM

JessiahC, on 04 December 2013 - 10:43 AM said:
-code snip-

Well in that case, now that I 100% understand what you were doing, here is the refactor


local function plotPos() --# there was still no used of num in the function, it was an unused variable
  local plotter, plotW, plotR = energyFlow / 300 --# we don't need to define what plotW and plotR here, this is just a forward declaration to make them local variables, they're auto assigned nil
  plotW, plotR = math.modf(plotter) --# math.modf will separate the parts out, first return value is the integral number, the second return is the fractional number, negatives inclusive; see: http://lua-users.org/wiki/MathLibraryTutorial
  plotR = round(plotR * 10, 1) --# use the round function I supplied in the "some areas I'd improve if I were doing myself" script
  if plotR >= 5 then
	plotW = plotW + 1
  end
  return plotW --# no need to do any negative conversions, the negatives were retained
end

Edited on 04 December 2013 - 05:21 PM

JessiahC #7

10 posts

Posted 04 December 2013 - 06:32 PM

theoriginalbit, on 04 December 2013 - 06:19 PM said:
-snip-

That's awesome, thanks!