Bad code optimization?

Help and Feedback Code Review
#1

(No one replied to my last post so I am posting it again) As the title says i want you guys to see if this is a bad way of doing this and if there is a better way of optimizing it
(Each slot is for a different setting btw)

for _, settingUis in SettingUi:GetChildren() do
	if settingUis:IsA("Frame") then
		if settingUis.Name == "Slot1" then
			settingUis.SwitchFrame.SwitchButton.Activated:Connect(function()
				if Configs.Value.Value == false then
					GuiController.Slide(SettingUi.Slot1.SwitchFrame.SwitchButton, "Off")
					SettingUi.Slot1.SwitchFrame.BackgroundColor = BrickColor.new("Really red")
					SettingUi.Slot1.SwitchFrame.SwitchButton.BackgroundColor = BrickColor.new("Crimson")
					Configs.Value.Value = true
				else
					GuiController.Slide(SettingUi.Slot1.SwitchFrame.SwitchButton, "On")
					SettingUi.Slot1.SwitchFrame.BackgroundColor = BrickColor.new("Lime green")
					SettingUi.Slot1.SwitchFrame.SwitchButton.BackgroundColor = BrickColor.new("Forest green")
					Configs.Value.Value = false
				end
			end)
		end
	end
	if settingUis.Name == "Slot2" then
		settingUis.SwitchFrame.SwitchButton.Activated:Connect(function()
			if Configs.Value.Value == false then
				GuiController.Slide(SettingUi.Slot2.SwitchFrame.SwitchButton, "Off")
				SettingUi.Slot2.SwitchFrame.BackgroundColor = BrickColor.new("Really red")
				SettingUi.Slot2.SwitchFrame.SwitchButton.BackgroundColor = BrickColor.new("Crimson")
				Configs.Value.Value = true
			else
				GuiController.Slide(SettingUi.Slot2.SwitchFrame.SwitchButton, "On")
				SettingUi.Slot2.SwitchFrame.BackgroundColor = BrickColor.new("Lime green")
				SettingUi.Slot2.SwitchFrame.SwitchButton.BackgroundColor = BrickColor.new("Forest green")
				Configs.Value.Value = false
			end
		end)
	end
	
	if settingUis.Name == "Slot3" then
		settingUis.SwitchFrame.SwitchButton.Activated:Connect(function()
			if Configs.Value.Value == false then
				GuiController.Slide(SettingUi.Slot3.SwitchFrame.SwitchButton, "Off")
				SettingUi.Slot3.SwitchFrame.BackgroundColor = BrickColor.new("Really red")
				SettingUi.Slot3.SwitchFrame.SwitchButton.BackgroundColor = BrickColor.new("Crimson")
				Configs.Value.Value = true
			else
				GuiController.Slide(SettingUi.Slot3.SwitchFrame.SwitchButton, "On")
				SettingUi.Slot3.SwitchFrame.BackgroundColor = BrickColor.new("Lime green")
				SettingUi.Slot3.SwitchFrame.SwitchButton.BackgroundColor = BrickColor.new("Forest green")
				Configs.Value.Value = false
			end
		end)
	end
end
1 Like
#2

Why not just…

local function setupSwitchButton(slotFrame: Frame, configValue: BoolValue)
	slotFrame.SwitchFrame.SwitchButton.Activated:Connect(function()
		if not configValue.Value then
			GuiController.Slide(slotFrame.SwitchFrame.SwitchButton, "Off")
			slotFrame.SwitchFrame.BackgroundColor = BrickColor.new("Really red")
			slotFrame.SwitchFrame.SwitchButton.BackgroundColor = BrickColor.new("Crimson")
			configValue.Value = true
		else
			GuiController.Slide(slotFrame.SwitchFrame.SwitchButton, "On")
			slotFrame.SwitchFrame.BackgroundColor = BrickColor.new("Lime green")
			slotFrame.SwitchFrame.SwitchButton.BackgroundColor = BrickColor.new("Forest green")
			configValue.Value = false
		end
	end)
end

for _, settingUis in SettingUi:GetChildren() do
	if settingUis:IsA("Frame") then
		if settingUis.Name == "Slot1" then
			setupSwitchButton(SettingUi.Slot1, Configs.Value)
		elseif settingUis.Name == "Slot2" then
			setupSwitchButton(SettingUi.Slot2, Configs.Value)
		elseif settingUis.Name == "Slot3" then
			setupSwitchButton(SettingUi.Slot3, Configs.Value)
		end
	end
end
#3

This would be the same result. I am looking for more optimized methods on doing this.

#4

I was wondering… How about the usage of tags?

#5

To be honest I really don’t see a different from the code now or if I use tags…

#6

Me either lol… I tried my best and cleaned your code up a bit. Enjoy!

#7

Thank you so much(300000000000000)

#8

Hey I took your advice and it turned out really well! Thank you so much.

#9

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.