Could this be made better?

Help and Feedback Code Review
#1

Simply what I’m doing is making a lock key system, it works fine already, but I’m seeing if it can be improved. Code:

local nums = {}

local gui = game:GetService('Workspace').NumPad.SurfaceGui
local code = gui:WaitForChild('CODE').Value

local function detect()
	if nums ~= nil then
		return tonumber(table.concat(nums,""))
	end
end

local function ClearTable(tbl)
	table.clear(tbl)
end

for i,v in pairs(gui:GetChildren()) do
	if v:IsA('TextButton')	then 
		if v.Name ~= 'Clear' and v.Name ~= 'Enter' then
			v.Activated:Connect(function()
				table.insert(nums,tonumber(v.Name))
				print(detect())
			end)
		elseif v.Name == 'Clear' then
			v.Activated:Connect(function()
				ClearTable(nums)
			end)
		elseif v.Name == 'Enter' then
			v.Activated:Connect(function()
				if detect() == code then
					--Does code if successfull.
					ClearTable(nums)
				else
					ClearTable(nums)
				end
			end)
		end
	end
end
1 Like
#2

A few things. First of all, your function capitalization is inconsistent: Decide between PascalCase or camelCase.

Second of all, you use a for loop to iterate through the gui’s children. GetChildren returns a numerically-indexed table, so in this case you should use ipairs instead of pairs. Better practice.

Another small thing I noticed was that you used the names “i” and “v” for the for loop. You did not use the index variable at all (i), so consider changing it to _.

Also, i and v can be confusing when reading over code quickly. Try renaming the value variable (v) to child. You’ll be able to read it faster as you understand “child” as the child of the gui variable.

Last of all, your code inside of the for loop can be improved. Lua does not have a case-switch built-in, so I like to implement it myself very simply.

You can use modulescripts for increased modularity (too overkill in this situation) or use a simple dictionary and check for matches.

An example:

for _, child in ipairs(gui:GetChildren()) do
    if (not guiFunctions[child.Name]) then continue end
    guiFunctions[child.Name]()
end

You can also have a default one that replaces the first statement in the for loop (the one that says continue).

An example:

for _, child in ipairs(gui:GetChildren()) do
    if (not guiFunctions[child.Name]) then
        — do something here
        continue
    end

    guiFunctions[child.Name]()
end
1 Like