5 replies to this topic

[Himself12794]

So I made another code for my turtle OS, and, again, just know that it can be made so much shorter and so much more nicer looking. I finished the information gathering functions, and I think they look horrible (at least they work!), so once I again I ask you all what I can do to improve it, things I can do to make it shorter, etc.

The reason I always ask is because all of my coding is self-taught and I have no idea what a professional code should look like.

Here's the code, which gathers the information needed for constructing an Industrial Blast Furnace:

Spoiler

local diggingMode = false

local function lineClear()
	x,y=term.getCursorPos()
	term.clearLine()
	term.setCursorPos(1,y)
end

local function dichotomy(a,B)/> --Gives a selection of two choices, a or b, returning true for a and false for b
	sel=true
	while true do
		if sel then
			write("--> "..a.."	 "..B)/>
		else
			write("	"..a.." --> "..B)/>
		end
		event,key=os.pullEvent()
		if event=="key" and key==203 then sel=not sel end
		if event=="key" and key==205 then sel=not sel end
		if event=="key" and key==28 then break end
		sleep(0.01)
		lineClear()
	end
	print()
	return sel
end

local function enableDiggingMode()-- Asks to enable digging mode
	print('Do you wish to enable digging mode?')
	print('If you do so, the turtle will clear a 3x3x4 area for the casing.')
	print('Turtle with digging tool required. (Obviously)')
	print()
	print('Enable digging mode?')
	if dichotomy('Yes','No') then
		diggingMode=true
	else
		diggingMode=false
	end
end

local function placeSlot(side,slot,wait)--Places the items in slot 'slot' on side 'side', and will wait for an item to be available if wait is true, otherwise does not check availability
	if turtle.getItemCount(slot)==0 and wait then
		print('Insufficient resources to complete build. Please place an item in slot '..tostring(slot))
		while turtle.getItemCount(slot)==0 do
			sleep(0.01)
		end
	end
	turtle.select(slot)
	placeSide(side)
end

local function placeSide(side)-- Places a block on side left, right, or back
	if not side then
		turtle.place()
	elseif side=='left' then
		turtle.turnLeft()
		turtle.place()
		turtle.turnRight()
	elseif side=='right' then
		turtle.turnRight()
		turtle.place()
		turtle.turnLeft()
	elseif side=='back' then
		turtle.turnRight()
		turtle.turnRight()
		turtle.place()
		turtle.turnLeft()
		turtle.turnLeft()
	end
end

local function countResources()--Counts turtle inventory items
	local count=0
	local i=1
	for i, 16 do
		count=count+turtle.getItemCount(i4)
	end
	return count
end

local function keyToContinue()--Stalls the screen until a key is pressed
	write('Press any key to continue')
	_=os.pullEvent('key')
	lineClear()
end

local function calculateHeat()--Gets the projected heat of Blast Furnace Based on configuration of different casings and upgrades in turtle inventory
	local value={
		standard=30,
		reinforced=50,
		advanced=70,
		extraneous=500
	}
	while true do
		os.startTimer(0.05)
		local event,key=os.pullEvent()
		if event=='timer' then
			local count={
				standard=turtle.getItemCount(1),
				reinforced=turtle.getItemCount(2),
				advanced=turtle.getItemCount(3),
				lava=turtle.getItemCount(4)+turtle.getItemCount(5)==2,
				nichrome=turtle.getItemCount(6)>=4,
				kanthal=turtle.getItemCount(7)>=4
			}

			local casings=count['standard']+count['reinforced']+count['advanced']
			
			if count['lava'] then count['lava']=1 else count['lava']=0 end
			if count['nichrome'] then count['nichrome']=1 else count['nichrome']=0 end
			if count['kanthal'] then count['kanthal']=1 else count['kanthal']=0 end
			
			local requiredHeat=(value['standard']*count['standard'])+(value['reinforced']*count['reinforced'])+(value['advanced']*count['advanced'])
			local extraneousHeat=(count['lava']*value['extraneous'])+(count['nichrome']*value['extraneous'])+(count['kanthal']*value['extraneous'])
			heat=requiredHeat+extraneousHeat
			lineClear()
			if casings>34 and heat>3880 then
				write(3880)
				write(' K')
				write('  Too many casings! Only 34!')
			elseif casings>34 and heat<=3880 then
				write(heat)
				write(' K')
				write('  Too many casings! Only 34!')
			elseif casings<34 then
				write(heat)
				write(' K')
				write('  Not enough casings! 34 Needed!')
			else
				write(heat)
				write(' K')
			end
			
		elseif event=='key' then
			return heat
		end
	end
end

local function info()--Gives basic overview of requirements and upgrades for the Industrial Blast Furnace
	term.clear()
	term.setCursorPos(1,1)
	enableDiggingMode()
	term.clear()
	term.setCursorPos(1,1)
	print('In an industrial blast furnace, the temperature is based on the type of machine casings you use, if you have heating coil upgrades and what type are they, and if you have lava in the middle.')
	print()
	print('The furnace requires 34 machine casings to complete. Heating coil upgrades are not required and can be added later. '..
	'Lava neither is required, but is highly recommended.')
	keyToContinue()
	term.clear()
	term.setCursorPos(1,1)
	write('Put standard casings in slot 1, reinforced in 2, and advanced in 3. '..
	'Place lava in slots 4 & 5. For nichrome coil upgrades, place 4 in '..
	'slot 6. For kanthal coil upgrades, place 4 in slot 7. Finally, place the Industrial Blast Furnace block in slot 8\n\n'..
	'The heat is calculated below:')
	term.setCursorPos(1,13)
	--write('Note: Max heat is 3880, although there is no reason for the heat to be any more than 3000 K.\n')
	write('Press any key to continue')
	term.setCursorPos(1,10)
	local heat,count=calculateHeat()
	--os.reboot()
	--print('Note: Max heat is 3880, although there is no reason for the heat to be any more than 3000 K.')
	return heat
end

local function beginAction(heat)--Looks at the heat and item count, and determines if construction can begin, and gives choice to start regardless of resource availability	
	term.clear()
	term.setCursorPos(1,1)
	local count={
		standard=turtle.getItemCount(1),
		reinforced=turtle.getItemCount(2),
		advanced=turtle.getItemCount(3),
		lava=turtle.getItemCount(4)+turtle.getItemCount(5)==2,
		nichrome=turtle.getItemCount(6)>=4,
		kanthal=turtle.getItemCount(7)>=4
	}
	
	local casings=count['standard']+count['reinforced']+count['advanced']
	
	write('Construction will begin with '..count['standard']..' standard casings, '..count['reinforced']..' reinforced, '..
	count['advanced']..' advanced casings, ')
	if count['nichrome'] then write('nichrome coil upgrades, ') end
	if count['kanthal'] then write('kanthal coil upgrades, ') end
	if count['lava'] then write('and lava,') else write('and no lava,') end
	write(' with a heat of '..tostring(heat)..' K.')
	print()
	print()
	
	if casings>34 or countResources()<35 then
		print('There is a discrepency in the number of required materials.')
		print()
		print('Continue anyway?')
		if dichotomy('Continue','Quit') then
			print('Construction will begin')
			sleep(1.5)
			term.clear()
			term.setCursorPos(1,1)
		else
			os.reboot()
		end
	else
		print('Begin contruction?')
		if dichotomy('Yes','Quit') then
			print()
			print('Construction will begin')
			sleep(1.5)
			term.clear()
			term.setCursorPos(1,1)
		else
			os.reboot()
		end
	end
end
local heat=info()
--term.clear()
--term.setCursorPos(1,1)
--print(heat)
--textutils.tabulate(count)
beginAction(heat)

Most of my focus is on making the code shorter and simpler, but input on code organization is fully welcome as well.
Hopefully, based on the repeated input I get, I'll need less and less help, and perhaps can provide some of my own.
Thanks in advance!

Edited by Himself12794, 06 February 2014 - 10:37 PM.

[theoriginalbit]

theres things throughout that you can fix. These are the 5 6 things that stood out to me immediately when I had a quick scan of your code.

#1 is localising your variables

#2 your for loops

local i=1
for i, 16 do
  count=count+turtle.getItemCount(i4)
end

since you do not use i outside the for loop once it is complete there is no need to perform a forward declaration. if you do it like so

for i = 1, 16 do
  count=count+turtle.getItemCount(i4)
end

i is localised to the for loop.

#3 This

if dichotomy('Yes','No') then
  diggingMode=true
else
  diggingMode=false
end

can simply be this

diggingMode = dichotomy('Yes', 'No')

conditionals are booleans, so why test a conditional to then assign a boolean, its just waste of a fork and CPU cycles.

#4 while waiting for the user to input items you can actually wait for the turtle_inventory event (assuming you're using CC 1.55 or above)

#5 these

if event=="key" and key==203 then sel=not sel end
if event=="key" and key==205 then sel=not sel end
if event=="key" and key==28 then break end

could actually just be

if event == "key" then
  if key == 203 or key == 205 then
	sel = not sel
  else if key == 28 then
	break
  end
end

this reduces the amount of conditionals that need to be evaluated by removing unnecessary event checking.

EDIT: Found another one
#6 this

if casings>34 and heat>3880 then
  write(3880)
  write(' K')
  write('  Too many casings! Only 34!')
elseif casings>34 and heat<=3880 then
  write(heat)
  write(' K')
  write('  Too many casings! Only 34!')
elseif casings<34 then
  write(heat)
  write(' K')
  write('  Not enough casings! 34 Needed!')
else
  write(heat)
  write(' K')
end

can be simplified to the following

write(math.min(heat, 3880)..' K')
if casings > 34 then
  write('  Too many casings! Only 34!')
elseif casings < 34 then
  write('  Not enough casings! 34 Needed!')
end

Edited by theoriginalbit, 07 February 2014 - 12:33 AM.

[Himself12794]

theoriginalbit, on 07 February 2014 - 12:15 AM, said:

theres things throughout that you can fix. These are the 5 6 things that stood out to me immediately when I had a quick scan of your code.

#1 is localising your variables

#2 your for loops

local i=1
for i, 16 do
  count=count+turtle.getItemCount(i4)
end

since you do not use i outside the for loop once it is complete there is no need to perform a forward declaration. if you do it like so

for i = 1, 16 do
  count=count+turtle.getItemCount(i4)
end

i is localised to the for loop.

#3 This

if dichotomy('Yes','No') then
  diggingMode=true
else
  diggingMode=false
end

can simply be this

diggingMode = dichotomy('Yes', 'No')

conditionals are booleans, so why test a conditional to then assign a boolean, its just waste of a fork and CPU cycles.

#4 while waiting for the user to input items you can actually wait for the turtle_inventory event (assuming you're using CC 1.55 or above)

#5 these

if event=="key" and key==203 then sel=not sel end
if event=="key" and key==205 then sel=not sel end
if event=="key" and key==28 then break end

could actually just be

if event == "key" then
  if key == 203 or key == 205 then
	sel = not sel
  else if key == 28 then
	break
  end
end

this reduces the amount of conditionals that need to be evaluated by removing unnecessary event checking.

EDIT: Found another one
#6 this

if casings>34 and heat>3880 then
  write(3880)
  write(' K')
  write('  Too many casings! Only 34!')
elseif casings>34 and heat<=3880 then
  write(heat)
  write(' K')
  write('  Too many casings! Only 34!')
elseif casings<34 then
  write(heat)
  write(' K')
  write('  Not enough casings! 34 Needed!')
else
  write(heat)
  write(' K')
end

can be simplified to the following

write(math.min(heat, 3880)..' K')
if casings > 34 then
  write('  Too many casings! Only 34!')
elseif casings < 34 then
  write('  Not enough casings! 34 Needed!')
end

Oh yes! That's exactly the type input I'm looking for. I'm going to start looking at more advanced users codes and perhaps I can learn some styling lessons. I'm gonna have to start crediting you in my codes that you help with.
Also, is there a way I can do the program without having to declare this twice?

    local count={
        standard=turtle.getItemCount(1),
        reinforced=turtle.getItemCount(2),
        advanced=turtle.getItemCount(3),
        lava=turtle.getItemCount(4)+turtle.getItemCount(5)==2,
        nichrome=turtle.getItemCount(6)>=4,
        kanthal=turtle.getItemCount(7)>=4
    }

I tried passing it out of the function like I did with heat, but it did not work, even when I removed the local statement from in front of it.

[theoriginalbit]

Himself12794, on 07 February 2014 - 12:45 AM, said:

I'm gonna have to start crediting you in my codes that you help with.

technically not needed since anyone who helps in Ask a Pro is freely giving help and advice, if they want credit they shouldn't be helping. but thank you for the sentiment.

Himself12794, on 07 February 2014 - 12:45 AM, said:

Also, is there a way I can do the program without having to declare this twice?

Yeah of course, just define it at the top of your program — just as you did with diggingMode. any function below its definition will have access to it.

[Himself12794]

theoriginalbit, on 07 February 2014 - 12:49 AM, said:

Himself12794, on 07 February 2014 - 12:45 AM, said:

Also, is there a way I can do the program without having to declare this twice?

Yeah of course, just define it at the top of your program — just as you did with diggingMode. any function below its definition will have access to it.

I was going to do that, but if I do I can't get the update if the amount of items change. I have it so it changes every time there is a difference in the inventory.

Edited by Himself12794, 07 February 2014 - 12:57 AM.

[theoriginalbit]

Himself12794, on 07 February 2014 - 12:55 AM, said:

I was going to do that, but if I do I can't get the update if the amount of items change. I have it so it changes every time there is a difference in the inventory.

you could make a little function to refresh the values inside of the table, which you invoke when you want the latest info.