How To Clean Code?

Help and Feedback Scripting Support
#1

Hello my name is Juice_ED and I am fairy new scripter and I don’t know how to make my code better/clean it, I have searched youtube’s video and nothing came up, here is some of my code (I know it’s bad)

local player = game.Players.LocalPlayer
local part = workspace.Checker.Part
local Money = workspace.Money

game.Players.PlayerAdded:Connect(function(player)
	
	local leaderstats = Instance.new("Folder")
	leaderstats.Name = "leaderstats"
	leaderstats.Parent = player

	local gold = Instance.new("IntValue")
	gold.Name = "Cash"
	gold.Value = 0
	gold.Parent = leaderstats
	
	
end)

part.Touched:Connect(function(hit) 

	player = game.Players:FindFirstChild(hit.Parent.Name) 
	
	if player then 
		
		local leaderstats = player:WaitForChild("leaderstats")
		local Cash = leaderstats:FindFirstChild("Cash")

		Cash.Value = Cash.Value + Money.Value.Value
		
		Money.Value.Value = 0 

	end
end)


local BillboardGUI = script.Parent.RedPart.BillboardGui
local RepliactedStorage = game:WaitForChild("ReplicatedStorage")
local Wall = RepliactedStorage.Wall2.Part
local MoneyValue = script.Parent.Value.Value
local NameValue = script.Parent.NameValue.Value
local player = game.Players.LocalPlayer

BillboardGUI.Text.Text = NameValue.. " Cost: "..MoneyValue 


script.Parent.RedPart.Touched:Connect(function(hit)

	player = game.Players:FindFirstChild(hit.Parent.Name) 
	if player then 
		local leaderstats = player:WaitForChild("leaderstats")
		local Cash = leaderstats:FindFirstChild("Cash")

		if Cash.Value >= MoneyValue  then

			Cash.Value = Cash.Value - MoneyValue

			local ClonedWall = Wall:Clone()
			ClonedWall.Parent = workspace
			ClonedWall.Position = Vector3.new(-71.201, 1.5, 4.674)
			script.Parent.RedPart:Destroy()
			script.Parent.Part:Destroy()

		else 

			print("Not Enough")
			BillboardGUI.Text.Text = "Not Enough"		
			wait(5)
			BillboardGUI.Text.Text = NameValue.." Cash: "..MoneyValue


		end


	end
end)


 

local BillboardGUI = script.Parent.RedPart.BillboardGui
local RepliactedStorage = game:WaitForChild("ReplicatedStorage")
local Upgarder = game.ReplicatedStorage.Upgrader2
local MoneyValue = script.Parent.Value.Value
local NameValue = script.Parent.NameValue.Value
local player = game.Players.LocalPlayer

BillboardGUI.Text.Text = NameValue.. " Cost: "..MoneyValue 

script.Parent.RedPart.Touched:Connect(function(hit)

	player = game.Players:FindFirstChild(hit.Parent.Name) 
	if player then 
		local leaderstats = player:WaitForChild("leaderstats")
		local Cash = leaderstats:FindFirstChild("Cash")

		if Cash.Value >= MoneyValue  then

			Cash.Value = Cash.Value - MoneyValue

			local ClonedUpgarder = Upgarder:Clone()
			ClonedUpgarder.Parent = workspace
			
			
			script.Parent.RedPart:Destroy()
			script.Parent.Part:Destroy()

		else 

			print("Not Enough")
			BillboardGUI.Text.Text = "Not Enough"		
			wait(5)



		end


	end
end)
1 Like
#2

don’t over space like this:

also use easier to read variable names so others can understand your code better

2 Likes
#3

First of all this should go on #help-and-feedback:code-review. Apart from that you can optimize and give a better look to the code recycling with variables and formating your code correctly, you can use this tool which makes it automatically for you.
image

Don’t use global variables without any reason, it’s not optimal (this also aplies for functions).

As @TheLordGamer1820 don’t overspace.

Do not repeat code, instead use variables and with this example it’s more optimal to use :GetService() when calling a service.

local Players = game:GetService("Players")

local Player = Players.LocalPlayer

Players.PlayerAdded:Connect(function() end)

I think that’s all. You’re going in the right way, wish you the best of luck and hope you continue programming. :slight_smile:

3 Likes
#4

The order that you put statements can make a big difference in terms of readability. As a general rule I like to put GetService calls at the top, then require calls, then constants, then non-constant variables that need to be in the outermost scope, then functions and then Connect calls.

Another thing you could improve is the case you write variables in. I like to use PascalCase for services, as well as classes or modules that are retrieved from ModuleScripts. I use UPPER_CASE for constants, and snakeCase for most other variables. What you do isn’t important, as long as you do it consistently.

Choosing names is hard and the only way to learn it is to think about it and practice doing it. For example, part is a bad variable name because it only tells you which type of thing it is, nothing about what it is used for. moneyCollectorTrigger would be a better name, assuming you rename “checker” to “moneyCollector” (that’s what it seems to be doing?).

Your code also doesn’t seem to work, since you’re making critical changes from a LocalScript that should really be done from a server Script. Look up the client/server model as well as scripting with remoteevents on the wiki for more info.

1 Like