Feedback on my code

Help and Feedback Code Review
#1

I need feedback on my code. Is it efficient, what can I do to improve it?

local ReplicatedStorage = game:GetService("ReplicatedStorage")
local Players = game:GetService("Players")

local Player = Players.LocalPlayer
local PlayerGui = Player:WaitForChild("PlayerGui")

local ShopUI = PlayerGui:WaitForChild("ShopUI")
local SideUI = PlayerGui:WaitForChild("SideUI")
local UpgradeUI = PlayerGui:WaitForChild("UpgradeUI")

local MainFrame = ShopUI:WaitForChild("Frame")
local Categories = MainFrame.Categories.ButtonBar

local PointsFrame = MainFrame.Points.PointsFrame
local ItemsFrame = MainFrame.Items.ItemsFrame.ScrollingFrame
local PassesFrame = MainFrame.Passes.GamepassFrame
local ViewFrames = MainFrame.ViewFrames

local function HideFrames()	
	for _, ViewFrames in ipairs(ViewFrames:GetChildren()) do
		ViewFrames.Visible = false
	end
 	PointsFrame.Visible = false
	ItemsFrame.Visible = false
	PassesFrame.Visible = false
end

local function ViewFrames(Frame)
	HideFrames()
	Frame.Visible = true
end

local function changeColorDefault()
	for _, Category in ipairs(Categories:GetChildren()) do
		Category.ImageColor3 = Color3.fromRGB(73, 83, 85)
	end
end

local function changeColor(Frame)
	changeColorDefault()
	Frame.ImageColor3 = Color3.fromRGB(115, 188, 214)
end

for _, Category in ipairs(Categories:GetChildren()) do
	
	Category.Button.Activated:Connect(function()
		
    if Category.Name == "Items" then
			ReplicatedStorage.Audio.ClickSound:Play()
			changeColor(Category)
	        ViewFrames(ItemsFrame)
		
	elseif Category.Name == "Passes" then
			ReplicatedStorage.Audio.ClickSound:Play()
			changeColor(Category)
	        ViewFrames(PassesFrame)
		
	elseif Category.Name == "Points" then
			ReplicatedStorage.Audio.ClickSound:Play()
			changeColor(Category)
	        ViewFrames(PointsFrame)
		 end
	end)
end

return HideFrames

Note: It’s a module that returns 1 (using it for organization)

2 Likes
#2

You have a syntax error at the top where you write “llocal” instead of “local”

Otherwise, seems fine, but why would the module return 1 instead of the HideFrames function? Makes it seem kind of pointless for it to exist in the first place if it doesn’t actually do anything.

3 Likes