Jump to content




Help with Clean-up of code for a little turtle OS with building templates

Started by Himself12794, Jan 27 2014 05:22 PM
turtle lua

  • You cannot reply to this topic
8 replies to this topic

#1 Himself12794

  • Members
  • 56 posts

Posted 27 January 2014 - 05:22 PM

I'm currently working on a turtle OS that will have some useful programs. So far, what I have works, but I can't help but think that the code could be shorter and a little less complicated. Any tips on simplification is appreciated. There's three codes so far that I have.

The startup code, which displays all the programs in a directory as multiple choice options, with configurable visibility:
Spoiler

Then the tower code, which just builds a square tower with configurable width and height:
Spoiler

And finally a program that will build a gregtech industrial grinder, and dig out the area for it if requested:
Spoiler

I'm relatively new to coding, so I haven't got the ability for aesthetically pleasing code yet. Any suggestions or tips are greatly appreciated!

#2 CometWolf

  • Members
  • 1,283 posts

Posted 27 January 2014 - 05:46 PM

Some indentation and simple comments go a long way when it comes to making nice code. That is, simple as in only explain the more complex/weird/strange things you do. I usually structure my code into sections, one for variables/tables and the like, another for functions and finally one for the actual program. These sections should ofcourse be commented to mark what is what in a clean and nice way.

Have a look at my currently largest program to see what i mean
http://pastebin.com/Kt4D8uyE
My cleanliness does suffer a bit as it goes on though, lol.

As for the actual function of your code, i'll take a look tomorrow, since im going to bed now :P

#3 Himself12794

  • Members
  • 56 posts

Posted 27 January 2014 - 06:18 PM

View PostCometWolf, on 27 January 2014 - 05:46 PM, said:

Some indentation and simple comments go a long way when it comes to making nice code. That is, simple as in only explain the more complex/weird/strange things you do. I usually structure my code into sections, one for variables/tables and the like, another for functions and finally one for the actual program. These sections should ofcourse be commented to mark what is what in a clean and nice way.

Have a look at my currently largest program to see what i mean
http://pastebin.com/Kt4D8uyE
My cleanliness does suffer a bit as it goes on though, lol.

As for the actual function of your code, i'll take a look tomorrow, since im going to bed now :P
Well I am nowhere near that level of expertise, but that looks magnificent. All I know about coding has come from doing computercraft.

#4 CometWolf

  • Members
  • 1,283 posts

Posted 28 January 2014 - 10:47 AM

I gave your first program a quick once over. afew things to note is to always use local variables unless you have a good reason not to. It's also good practice to start variable names with lower case of the first letter of the type (t for table, b for boolean, n for number, s for string). Personally i just do this for tables though :P. While we're on the subject of variables, if you're getting multiple variables from a function which you don't intend to use, start their name with an underscore.

A neat trick to keep in mind when it comes to function arguments is that when you're checking the variable for nil(not used) you can do the following
local function test(variable)
  variable = variable or derp
  print(variable)
end
This will print the variable if it is given, otherwise it will print derp

Another thing to note is how you use your if statements. If you're checking the same condition multiple times, in this case your key presses, you should put the sub conditions within the if statement instead. Also, when you're checking the same variable for different values, use elseif statements.
Finally, you should know that return will end the function anyways, as such the break which then goes to a return is not really nessacary. Neither is sleep in a os.pullEvent loop, as os.pullEvent yields anyways.
--tables and variables
local choice = fs.list('')
local newTable = fs.list('')
local hidden = {
  'startup',
  'rom',
  'KeyNumb'
}
--functions
function hideValues(array,hidden)
  for i,v in ipairs(array) do
    for i2,v2 in ipairs(hidden) do
	  if v==v2 then
		 table.remove(array, i)
		 return hideValues(array,hidden) --this is called a recursive call, and should be avoided where possible. There is probably a better way to do this, but i haven't really gotten into it.
	   end
	  end
    end
  end
end
 
local function lineClear()
  local _x,y=term.getCursorPos()
  term.clearLine()
  term.setCursorPos(1,y)
end
 
local function printCentered(str)
  local termX, _termY=term.getSize()
  local _cursX, cursY=term.getCursorPos()
  local newX = math.ceil(termX/2)-math.ceil(string.len(str)/2)
  term.setCursorPos(newX,cursY)
  print(str)
end
 
local function replaceDelim(str, delim)
  local result = ''
  for i in string.gmatch(str, '%a+') do
    result=result..i..delim
  end
  return result
end
 
local function spacifyTable(array)
  for i,v in ipairs(array) do
    array[i]=replaceDelim(v,' ')
  end
end
 
local function getArraySize(array)
  local count = 0
  for _ in ipairs(array) do
    count=count+1
  end
  return count
end

local function header()
  term.clear()
  term.setCursorPos(1,1)
  print("HimCo Industries' Turtley 2.0")
  print()
  printCentered('Select a template:')
  print()
end
 
local function multipleChoice(choices, sel)
  local size = getArraySize(choices)
  sel = sel or 1 --initial selection
  while true do
    --render screen
    header()
    for i,v in ipairs(choices) do
	  if i == sel then
	    printCentered('[ '..v..']')
	  else
	    printCentered('  '..v..' ')
	  end
    end
    --user input
    local event,key=os.pullEvent('key')
    if key == 208 --down arrow
    or key == 31 then -- S
	  sel = sel+1
	  if sel > size then
	    sel = 1 --botom reached, moving back to top
	  end
    elseif key == 200 -- up arrow
    or key == 17 then -- W
	  sel = sel-1
	  if sel < 1 then
	    sel = size -- top reached, moving to bottom
	  end
    elseif key == 28 -- enter
    or key == 57 then --space
	  return sel --input complete
    end
  end
end
 
--actual program
hideValues(choice,hidden)
spacifyTable(choice)
file=multipleChoice(choice)
hideValues(newTable,hidden)
shell.run(newTable[file])

#5 Himself12794

  • Members
  • 56 posts

Posted 28 January 2014 - 01:25 PM

View PostCometWolf, on 28 January 2014 - 10:47 AM, said:

I gave your first program a quick once over. afew things to note is to always use local variables unless you have a good reason not to. It's also good practice to start variable names with lower case of the first letter of the type (t for table, b for boolean, n for number, s for string). Personally i just do this for tables though :P. While we're on the subject of variables, if you're getting multiple variables from a function which you don't intend to use, start their name with an underscore.

A neat trick to keep in mind when it comes to function arguments is that when you're checking the variable for nil(not used) you can do the following
local function test(variable)
  variable = variable or derp
  print(variable)
end
This will print the variable if it is given, otherwise it will print derp

Another thing to note is how you use your if statements. If you're checking the same condition multiple times, in this case your key presses, you should put the sub conditions within the if statement instead. Also, when you're checking the same variable for different values, use elseif statements.
Finally, you should know that return will end the function anyways, as such the break which then goes to a return is not really nessacary. Neither is sleep in a os.pullEvent loop, as os.pullEvent yields anyways.
Spoiler
For the code:
function hideValues(array,hidden)
  for i,v in ipairs(array) do
	for i2,v2 in ipairs(hidden) do
		  if v==v2 then
				 table.remove(array, i)
				 return hideValues(array,hidden)
		   end
	  end
	end
  end
end
The only way I could get it to work was to call it recursively. Otherwise, if there were two values in a row to be hidden, it would skip the second one and not remove it.

Edited by Himself12794, 28 January 2014 - 01:29 PM.

#6 surferpup

  • Members
  • 286 posts
  • LocationUnited States

Posted 28 January 2014 - 01:29 PM

Is there a question here, or are you just re-quoting CometWolf?

#7 CometWolf

  • Members
  • 1,283 posts

Posted 28 January 2014 - 02:55 PM

He's responding to the comment here

Quote

function hideValues(array,hidden)
for i,v in ipairs(array) do
for i2,v2 in ipairs(hidden) do
if v==v2 then
table.remove(array, i)
return hideValues(array,hidden) --this is called a recursive call, and should be avoided where possible. There is probably a better way to do this, but i haven't really gotten into it.
end
end
end
end
end
The reason it skips one if you don't do it recursively is because you are using table.remove to remove your table indexes. This will remove the index in question, and if the table is numerically indexed it will also move all the following keys down one number. if you remove 4, 5 would now become the new 4. Thus when it checks number 5, it will actually be number 6.
Use this instead
function hideValues(array,hidden)
  for i,v in ipairs(array) do
	for i2,v2 in ipairs(hidden) do
		  if v==v2 then
				 array[i] = nil
		   end
		  end
	end
  end
end
If you wish to learn more, the difference between these 2 methods were recently discussed here:
http://www.computerc...t-from-a-table/

Edited by CometWolf, 28 January 2014 - 02:56 PM.

#8 Himself12794

  • Members
  • 56 posts

Posted 01 February 2014 - 11:23 AM

View PostCometWolf, on 28 January 2014 - 02:55 PM, said:

He's responding to the comment here

Quote

function hideValues(array,hidden)
for i,v in ipairs(array) do
for i2,v2 in ipairs(hidden) do
if v==v2 then
table.remove(array, i)
return hideValues(array,hidden) --this is called a recursive call, and should be avoided where possible. There is probably a better way to do this, but i haven't really gotten into it.
end
end
end
end
end
The reason it skips one if you don't do it recursively is because you are using table.remove to remove your table indexes. This will remove the index in question, and if the table is numerically indexed it will also move all the following keys down one number. if you remove 4, 5 would now become the new 4. Thus when it checks number 5, it will actually be number 6.
Use this instead
function hideValues(array,hidden)
  for i,v in ipairs(array) do
	for i2,v2 in ipairs(hidden) do
		  if v==v2 then
				 array[i] = nil
		   end
		  end
	end
  end
end
If you wish to learn more, the difference between these 2 methods were recently discussed here:
http://www.computerc...t-from-a-table/
Hmmm... when I do that, it seems to behave incorrectly. It refuses to show the Tower Builder option unless there are no files selected to be hidden. I'm not sure if this is due to placement of the file, or an error in my code.

#9 CometWolf

  • Members
  • 1,283 posts

Posted 05 February 2014 - 12:13 PM

Had to do some file hiding myself as part of something i made the other day. I found that forgoing the for loop in favor of a while true loop was the best option.
_G.nFs.tHide = {
  "startup",
},

_G.nFs.checkHidden = function(path)
  for k,v in pairs(_G.nFs.tHide) do
	if v == path then
	  return true
	end
  end
  return false
end

_G.nFs.list = function(path)
  local tFiles = _G.nFs.backup.list(path)
  local i = 1
  while true do
	if _G.nFs.checkHidden(tFiles[i]) then
	  table.remove(tFiles,i)
	elseif tFiles[i] == "startup2" then -- this elseif is irrelevant to what you're doing, so just ignore it :P/>/>/>/>
	  tFiles[i] = "startup"
	  i = i+1
	else
	  i = i+1
	end
	if i > #tFiles then
	  break
	end
  end
  return tFiles
end

Edited by CometWolf, 05 February 2014 - 12:15 PM.

Back to Ask a Pro


1 user(s) are reading this topic

0 members, 1 guests, 0 anonymous users



  1. ComputerCraft | Programmable Computers for Minecraft
  2. → ComputerCraft
  3. → Ask a Pro