Can y'all simplify/shorten this function?

Help and Feedback Code Review
#1 
for _, button in pairs(SolidModelingButtonsArray) do 
	button.MouseButton1Click:Connect(function()

		if debounce == true then
			return
		end

		debounce = true

		if button == UnionButton then
			
			local otherParts = SolidModelingFunctionsModule.GetOtherParts()

			if next(otherParts) == nil then
				print("more parts need to be selected")
				return
			end

			local success, newParts = pcall(function()
				return GeometryService:UnionAsync(CommonlyUsedVariablesModule.mainPart, otherParts, {SplitApart = false})
			end)

			if success and newParts then
				SolidModelingFunctionsModule.NewPartsFinishingTouches(newParts, otherParts)
			end
			
		elseif button == IntersectButton then
			
			local otherParts = SolidModelingFunctionsModule.GetOtherParts()

			if next(otherParts) == nil then
				print("more parts need to be selected")
				return
			end

			local success, newParts = pcall(function()
				return GeometryService:IntersectAsync(CommonlyUsedVariablesModule.mainPart, otherParts, {SplitApart = true})
			end)

			if success and newParts then
				SolidModelingFunctionsModule.NewPartsFinishingTouches(newParts, otherParts)
			end
			
		elseif button == SubtractButton then
			
			local otherParts = SolidModelingFunctionsModule.GetOtherParts()

			if next(otherParts) == nil then
				print("more parts need to be selected")
				return
			end

			local success, newParts = pcall(function()
				return GeometryService:SubtractAsync(CommonlyUsedVariablesModule.mainPart, otherParts, {SplitApart = true})
			end)

			if success and newParts then
				SolidModelingFunctionsModule.NewPartsFinishingTouches(newParts, otherParts)
			end
			
		elseif button == SeparateButton then
			
			SolidModelingFunctionsModule.CleanUp()

			local otherParts = SolidModelingFunctionsModule.GetOtherParts()

			SolidModelingFunctionsModule.RejuvenateChildren(CommonlyUsedVariablesModule.mainPart)

			for _, otherPart in pairs(otherParts) do
				SolidModelingFunctionsModule.RejuvenateChildren(otherPart)
			end

			ButtonSelectionFunctionsModule.SelectDoublePreviousToolBarButton()
			
		end

		task.wait(0.1)
		debounce = false
		return

	end)
end

Thanks!

#2

I’d say this is the perfect question to ask chatgpt. With that said you could probably just break this down into smaller functions so instead of having a for loop and then doing

for _.button in pairs()

button.MouseButton1Click:Connect(function()

end)

end

You could try doing

for _,button in pairs()
button.MouseButton1Click:Connect(onButtonPress)
end

And then really just determine what maybe should and shouldn’t be part of that function. You could also shorten things to a single line like if debounce then return end.

#3

Simplification code code can be done by finding repeated patterns within the code. In this case, within 3 of the 4 if statements, the following is the exact same:

image
image1971×1128 251 KB

We can condense that by changing the way the if statements are handled, like so:

if button == UnionButton or button == IntersectButton or button == SubtractButton then
	local otherParts = SolidModelingFunctionsModule.GetOtherParts()
	if next(otherParts) == nil then
		print("more parts need to be selected")
		return
	end
	local success, newParts = pcall(function()
		if button == UnionButton then
			return GeometryService:UnionAsync(CommonlyUsedVariablesModule.mainPart, otherParts, {SplitApart = false})
		elseif button == IntersectButton then
			return GeometryService:IntersectAsync(CommonlyUsedVariablesModule.mainPart, otherParts, {SplitApart = true})
		elseif button == SubtractButton then
			return GeometryService:SubtractAsync(CommonlyUsedVariablesModule.mainPart, otherParts, {SplitApart = true})
		end
	end)
	if success and newParts then
		SolidModelingFunctionsModule.NewPartsFinishingTouches(newParts, otherParts)
	end
elseif button == SeparateButton then
    ...

This can be further condensed through the use of tables, but wouldn’t recommend unless you’re comfortable with reading the self.method(self, ...) method syntax.

local operations = {
	[UnionButton] = GeometryService.UnionAsync,
	[IntersectButton] = GeometryService.IntersectAsync,
	[SubtractButton] = GeometryService.SubtractAsync,
}
if operations[button] then
	local otherParts = SolidModelingFunctionsModule.GetOtherParts()
	if next(otherParts) == nil then
		print("more parts need to be selected")
		return
	end
	local success, newParts = pcall(function()
		return operations[button](GeometryService, CommonlyUsedVariablesModule.mainPart, otherParts, {SplitApart = false})
	end)
	if success and newParts then
		SolidModelingFunctionsModule.NewPartsFinishingTouches(newParts, otherParts)
	end
elseif button == SeparateButton then
    ...

Which you’ll then notice, this syntax can just be thrown directly into the pcall. (since we’re returning the value of the operation anyways)

local success, newParts = pcall(function()
	return operations[button](GeometryService, CommonlyUsedVariablesModule.mainPart, otherParts, {SplitApart = false})
end)

-- can become:
local success, newParts = pcall(
	operations[button], 
	GeometryService, 
	CommonlyUsedVariablesModule.mainPart, 
	otherParts, 
	{SplitApart = false}
)
-- wouldn't recommend one line in this case

Some other notes:

if debounce == true then 
-- this can be replaced with
if debounce then

for _, button in pairs(SolidModelingButtonsArray) do
-- you can use the general Luau iterator
for _, button in SolidModelingButtonsArray do
-- has the benefit of doing the jobs of both ipairs and pairs so nothing ever breaks
Final Simplification 
for _, button in SolidModelingButtonsArray do 
	button.MouseButton1Click:Connect(function()
		if debounce then
			return
		end
		debounce = true
		
		local operations = {
			[UnionButton] = GeometryService.UnionAsync,
			[IntersectButton] = GeometryService.IntersectAsync,
			[SubtractButton] = GeometryService.SubtractAsync,
		}
		if operations[button] then
			local otherParts = SolidModelingFunctionsModule.GetOtherParts()

			if next(otherParts) == nil then
				print("more parts need to be selected")
				return
			end

			local success, newParts = pcall(
				operations[button], 
				GeometryService, 
				CommonlyUsedVariablesModule.mainPart, 
				otherParts, 
				{SplitApart = false}
			)

			if success and newParts then
				SolidModelingFunctionsModule.NewPartsFinishingTouches(newParts, otherParts)
			end
		elseif button == SeparateButton then
			SolidModelingFunctionsModule.CleanUp()

			local otherParts = SolidModelingFunctionsModule.GetOtherParts()
			SolidModelingFunctionsModule.RejuvenateChildren(CommonlyUsedVariablesModule.mainPart)

			for _, otherPart in otherParts do
				SolidModelingFunctionsModule.RejuvenateChildren(otherPart)
			end
			
			ButtonSelectionFunctionsModule.SelectDoublePreviousToolBarButton()
		end

		task.wait(0.1)
		debounce = false
	end)
end
1 Like
#4

you exited the function without setting debounce back to false.
consider

task.delay(0.1, function() debounce = false end)

instead of changing back at the function