Can this code be cleaner?

You can write your topic however you want, but you need to answer these questions:

  1. If possible can this code be better and cleaner

  2. I’m wondering if this code can be cleaner

  3. functions and parameters

for _, button in pairs(script.Parent.Parent.LeftMiddle:GetDescendants()) do
	if button:IsA("TextButton") then
		
		button.MouseEnter:Connect(function()
			if button.Name == "Inventory" then 
				EnterTween(Inventoryname,InfoEnter,GoalEnter)
				
			elseif button.Name == "Settings" then
				EnterTween(Settingsname,InfoEnter,GoalEnter)
				
			elseif button.Name == "Shop" then
				EnterTween(Shopname,InfoEnter,GoalEnter)
			end
		end)
		
		--[[ Mouse Exit Functions ]]--
		
		button.MouseLeave:Connect(function()
			if button.Name == "Inventory" then
				EnterTween(Inventoryname,InfoLeave,GoalLeave)
				
			elseif button.Name == "Settings" then
				EnterTween(Settingsname,InfoLeave,GoalLeave)
				
			elseif button.Name == "Shop" then
				EnterTween(Shopname,InfoLeave,GoalLeave)
			end
		end)
		
		--[[ Open And Close Functions ]]--
		
		button.MouseButton1Click:Connect(function()
			if not button.Active then return end
			button.Active = false
			ClickTween(button.Parent.Parent)
			if button.Name == "Inventory" then
				EnterTween(Inventoryname,InfoLeave,GoalLeave)
			elseif button.Name == "Settings" then
				EnterTween(Settingsname,InfoLeave,GoalLeave)
			elseif button.Name == "Shop" then
				EnterTween(Shopname,InfoLeave,GoalLeave)
			end
			task.wait(1.3)
			button.Active = true
		end)
	end
end
1 Like

I’m not sure if the category is open or not, but consider writing this inside of #help-and-feedback:code-review if the code already works.

  • If you’d like to, you can use ipairs instead of pairs because you are reading from an array not a dictionary.
  • I’m not sure what code portions have been removed here, but you can probably avoid these if statements by combining button.Name string & whatever {…}name is (I assume this reads from some table).
  • Unsure what task.wait() is, but wherever you build your Tween, if you use Tween.Completed:Wait() you may see better results in proper yielding of your code.

Task.wait() is basically the new way of saying wait(). It’s apart of the new Task Library introduced to developers. However as far as yielding the tween goes, I agree that Tween.Completed:Wait() should be used.

I could throw some lua tricks into it but it seems fine on the surface. As long as you can read it when you come back to it later on, it doesn’t really matter.

The only real thing I would look at is your Inventoryname Settingsname and Shopname variables - if you can just use the button.Name to reference to either one of these. Or handle it in the EnterTween function instead - and then you’re not repeating code within the for loop.

function EnterTween(name,infoEnter,goalEnter)
	name = name == "Inventory" and Inventoryname or name == "Settings" and Settingsname or name == "Shop" and Shopname
	--etc
end


for _, button in pairs(script.Parent.Parent.LeftMiddle:GetDescendants()) do
	if button:IsA("TextButton") then
		
		button.MouseEnter:Connect(function()
			EnterTween(button.Name,InfoEnter,GoalEnter)
		end)
		
		--[[ Mouse Exit Functions ]]--
		
		button.MouseLeave:Connect(function()
			EnterTween(button.Name,InfoLeave,GoalLeave)
		end)
		
		--[[ Open And Close Functions ]]--
		
		button.MouseButton1Click:Connect(function()
			if not button.Active then return end
			button.Active = false
			ClickTween(button.Parent.Parent)
			EnterTween(button.Name,InfoLeave,GoalLeave)
			task.wait(1.3)
			button.Active = true
		end)
	end
end

1 Like

Readability of your code is important, especially if you’re going to be actively maintaining the code or are working with other people who will read your code. Here are a couple things that could make your code a bit cleaner:

image


These screenshots are from the Roblox Lua Style guide:
https://roblox.github.io/lua-style-guide

I would argue that truthiness shouldn’t be used to set name here. What do you make of using a dict?

--TODO: Wherever Xname is set should likely be moved into here
local NAME_IDS = {
    Inventory = Inventoryname,
    Settings = Settingsname,
    Shop = Shopname
}

There are also a couple smaller comments I’d make, really nitpicky. I’d lose the nesting:

if not button:IsA("TextButton") then
    continue
end

I would also suggest using some utility function so you don’t have to create that nameless function.
(sidenote, why do we need to pass InfoLeave or GoalLeave? They seem to be definable before EnterTween?)

button.MouseLeave:Connect(Util.Call(EnterTween, button.Name, InfoLeave, GoalLeave))

I’ve already fix the code but yes what you said was correct thank you!

Love to see how it came out! :)))