Feedback on my code system

Help and Feedback Code Review
#1

Hi, I am using modular programming and functional programming and I just wanted to see what I could improve upon. This is the module script.

local module = {}
module.Code = {2,6,0,7}
function module.processInput(v, newCode, order, levelState)
	if not levelState then
		local num = tonumber(v.TextBox.Text)
		if num then
			newCode[order] = num
			if table.concat(module.Code) == table.concat(newCode) then
				return true
			end
		end
	end
end

return module

And here is the localscript

--Yellow1,Green2,Red3,White4
local codeModule = require(game:GetService("ReplicatedStorage").WhiteRoomCode)
local newCode = {}
local levelCompleted = false

local function completeLevel()
	levelCompleted = true
	print("Level Complete")
end

for _, v in pairs(script.Parent:GetChildren()) do
	if v:IsA("SurfaceGui") then
		v.TextBox.Changed:Connect(function(property)
			if v.TextBox == script.Parent.GreenSurfaceGuiWhiteRoom.TextBox then
				codeModule.processInput(v, newCode, 2, levelCompleted)
				if codeModule.processInput(v, newCode, 2, levelCompleted) then
					completeLevel()
				end
			elseif v.TextBox == script.Parent.RedSurfaceGuiWhiteRoom.TextBox then
				codeModule.processInput(v, newCode, 3, levelCompleted)
				if codeModule.processInput(v, newCode, 3, levelCompleted) then
					completeLevel()
				end
			elseif v.TextBox == script.Parent.WhiteSurfaceGuiWhiteRoom.TextBox then
				codeModule.processInput(v, newCode, 4, levelCompleted)
				if codeModule.processInput(v, newCode, 4, levelCompleted) then
					completeLevel()
				end
			elseif v.TextBox == script.Parent.YellowSurfaceGuiWhiteRoom.TextBox then
				codeModule.processInput(v, newCode, 1, levelCompleted)
				if codeModule.processInput(v, newCode, 1, levelCompleted) then
					completeLevel()
				end
			end
		end)
	end
end

Thanks for any suggestions!

2 Likes
#2

You should define services as game:GetService. So you wouldn’t have to define services each time you index something form them.

Since script.Parent:GetChildren() returns a array, ipairs is recommended to use. Also use GetPropertyChangedSignal over Changed since the latter fires (if the given argument) is changed while the second one fires whatever property is changed.

How I would write your local script:

--Yellow1,Green2,Red3,White4
local ReplicatedStorage = game:GetService("ReplicatedStorage")

local codeModule = require(ReplicatedStorage.WhiteRoomCode)

local newCode = {}
local levelCompleted = false

local function completeLevel()
	levelCompleted = true
	print("Level Complete")
end

for _, v in ipairs(script.Parent:GetChildren()) do
	if v:IsA("SurfaceGui") then
		v.TextBox:GetPropertyChangedSignal("Text"):Connect(function()
			if v.TextBox == script.Parent.GreenSurfaceGuiWhiteRoom.TextBox then
				codeModule.processInput(v, newCode, 2, levelCompleted)
				if codeModule.processInput(v, newCode, 2, levelCompleted) then
					completeLevel()
				end
			elseif v.TextBox == script.Parent.RedSurfaceGuiWhiteRoom.TextBox then
				codeModule.processInput(v, newCode, 3, levelCompleted)
				if codeModule.processInput(v, newCode, 3, levelCompleted) then
					completeLevel()
				end
			elseif v.TextBox == script.Parent.WhiteSurfaceGuiWhiteRoom.TextBox then
				codeModule.processInput(v, newCode, 4, levelCompleted)
				if codeModule.processInput(v, newCode, 4, levelCompleted) then
					completeLevel()
				end
			elseif v.TextBox == script.Parent.YellowSurfaceGuiWhiteRoom.TextBox then
				codeModule.processInput(v, newCode, 1, levelCompleted)
				if codeModule.processInput(v, newCode, 1, levelCompleted) then
					completeLevel()
				end
			end
		end)
	end
end

local module = {
  Code = {2, 6, 0, 7}
}

function module.processInput(v, newCode, order, levelState)
	if not levelState then
		local number = tonumber(v.TextBox.Text)
		if number and typeof(number) ==  "number"then
			newCode[order] = number
			if table.concat(module.Code) == table.concat(newCode) then
				return true
			end
		end
	end
end

return module

Also the function completeLevel only set’s a boolean to true, nothing else. Consider making it do something more.

1 Like