Writing clean code
Introduction
I see a lot of people writing messy code in their scripts, whether it’s a free model, a YouTube tutorial or a post here on the developer forum. Now, it may not seem that important to you, but I think it’s what shifts programming from a logical process, to an art. It also makes your scripts much more readable especially when your trying to debug a problem.
In this tutorial, I will be showing you how to write clean code, and improve your scripts aesthetics and readability.
What is covered
- Indentation
- Variable/Function names
- Organizing code
- Spacing
- Operator spacing
- Functions and modules
- Comments
How to write cleaner code
Before I start showing how I write cleaner code, I want to say that everyone’s “style” is different. In the end, you have to create your own way of writing code in the best way that works for you.
Indentation
First let’s talk about indentation. Now, most modern code editors do this automatically, but sometimes they don’t always do it correctly (though it’s rare for a modern editor to make a mistake). Anytime you need to see when a block of code starts and ends, you should always use one TAB or about 8 spaces. This will make it easier to see what code that block holds or debugging a simple problems such as missing closing parenthesis.
Take a look at this example. There are two versions of the same code. One clean and one messy. For the messy version I want you to find the problem without running the code to get an error message. Only use what you have here.
Messy code
local part=workspace.Part
part.Touched:Connect(function(h)
if h.Parent:FindFirstChild("Humanoid") then
if h.Parent.Humanoid.WalkSpeed>16 then
h.Parent.Humanoid.WalkSpeed=16
end
end)
Now there are more than one issue here (in terms of writing clean code) but just focus on the what will give you an error.
Did you find it? Even if you did, try looking at this one to see the difference:
Clean code
local part = workspace.Part
part.Touched:Connect(function(hit)
if hit.Parent:FindFirstChild("Humanoid") then
if hit.Parent.Humanoid.WalkSpeed > 16 then
hit.Parent.Humanoid.WalkSpeed = 16
end
end)
Now the solution to this is quite simple the code is missing an end
, but with more complex problems, your code being clean and readable is going to become more and more important.
Naming variables and functions
Variable names are very important when you have not looked at a script in a while and you have to change something. Let’s say you made a variable for how fast a part would move. You probably should call it speed
. Some people would call speed s
or spd
. This makes it hard to figure out what is going on even if you wrote it (especially when you have a large script with hundreds of variables). Always be as descriptive as you can with your variables.
Function names should let the programmer know what the function does before reading the code contained in it. A function that smoothly changes a value from a
to b
is called lerp (or linear interpolation). It wouldn’t be called L or LI. That could mean anything, L could be luck()
, LI could be lineImpact()
. The point is, you shouldn’t have to think about what the name stands for.
Organizing your code
This is something I do in all my scripts. I group specific parts of code together so I know what is what. For example, when I declare variables, all my services (DataStoreService, UserInputService, etc…) go at the top of the script. Next comes anything that holds an instance as a value, and finally, everything else, though those variables are grouped as well its just different for each script (you decide how those are grouped). I usually group them by type. Strings are grouped with any other string variable, numbers/floats are grouped with their type, etc (I usually group unassigned variables after assigned ones).
Variables within functions should be ordered the same as you order your global variables. This is to keep it consistent with the rest of your code. If your not sure what I mean by grouping, it is just a space between each group. Treat each function as a group meaning there is a space after the end
statement and the declaration of the next function or statement. Example:
Code
-- Group 1 (services)
local players = game:GetService("Players")
local runService = game:GetService("RunService")
-- Group 2 (instances)
local baseplate = workspace:FindFirstChild("Baseplate")
local model = workspace:FindFirstChild("Model 1")
-- Group 3 (integers)
local x = 1
local y = 2
local z = 3
-- Group 4 (nil/null/unassigned)
local one
local two
local three
For functions, they should be declared after all the global variables (can be accessed from any part of the script) but are still ordered. You should order your functions in such a way that if another function needs to invoke another function, it can. (without removing the local tag). Let’s say function x()
needs to invoke function y()
. Function x()
should be below function y()
so it can be invoked without problems. This means you can put local before all your functions! Example:
Code
-- services
-- variables
local function y()
-- code
end
local function x()
y() -- can call function y()
end
x()
When you want to invoke a function using events, I recommend you place those at the bottom of your script. I do this so that all my events are all in one place and all the functions are declared already.
An example of all these things would be my DataStore script I use is most of my unreleased projects. Don’t worry if you understand it or not just take a look at how things are organized:
DataStoreCode
local players = game:GetService("Players")
local dataStoreService = game:GetService("DataStoreService")
local dataStore = dataStoreService:GetDataStore("GameSaveV0.1a")
local tries = 3
local dataLoaded
local loadedData
local starterCash
-- Gets called/invoked from Get(plr)
local function Set(plr)
if dataLoaded then
-- This group is for data store related things
local key = "userCode-#plr" .. plr.UserId
local data = {
["Cash"] = starterCash,
["Level"] = 0,
["XP"] = 0
}
-- This group is used for securing the data
local count = 0
local success, err
repeat
success, err = pcall(function()
dataStore:SetAsync(key, data)
end)
count = count + 1
until success or count >= tries
if not success then
warn("Failed to set data. Error code: " .. tostring(err))
return
end
else
warn("Data has not been loaded. Please wait for data to be loaded. Plr name" .. plr.Name)
return
end
end
-- Gets invoked from CreateLeaderstats(plr) (grouping is the same as Set())
local function Get(plr)
local key = "userCode-#plr" .. plr.UserId
local count = 0
local success, err
repeat
success, err = pcall(function()
loadedData = dataStore:GetAsync(key)
end)
count = count + 1
until success or count >= tries
if not success then
warn("Failed to read data. Error code: " .. tostring(err))
plr:Kick("Failed to load data, please rejoin.")
return
end
if success then
if loadedData then
dataLoaded = true
return loadedData
else
dataLoaded = true
return {
["Cash"] = starterCash,
["Level"] = 0,
["XP"] = 0
}
end
end
end
local function CreateLeaderstats(plr)
local data = Get(plr)
local leaderstats = Instance.new("Folder")
leaderstats.Name = "leaderstats"
leaderstats.Parent = plr
local cash = Instance.new("IntValue")
cash.Name = "Cash"
cash.Parent = plr
local level = Instance.new("IntValue")
level.Name = "Level"
level.Parent = leaderstats
local xp = Instance.new("IntValue")
xp.Name = "XP"
xp.Parent = plr
cash .Value = data.Cash
level.Value = data.Level
xp.Value = data.XP
end
-- Event calls are all at the bottom (as well as a BindToClose() call)
players.PlayerAdded:Connect(CreateLeaderstats)
players.PlayerRemoving:Connect(Set)
game:BindToClose(function()
for i, v in next, players:GetChildren() do
if v then
Set(v)
end
end
end)
Spacing
Spacing also helps make the entire script overall just seem a bit cleaner. Lets decode my Set()
function in terms of spacing to help understand this.
Set() function
-- Gets called/invoked from Get(plr)
local function Set(plr)
if dataLoaded then
-- This group is for data store related things
local key = "userCode-#plr" .. plr.UserId
local data = {
["Cash"] = starterCash,
["Level"] = 0,
["XP"] = 0
}
-- This group is used for securing the data
local count = 0
local success, err
repeat
success, err = pcall(function()
dataStore:SetAsync(key, data)
end)
count = count + 1
until success or count >= tries
if not success then
warn("Failed to set data. Error code: " .. tostring(err))
return
end
else
warn("Data has not been loaded. Please wait for data to be loaded. Plr name" .. plr.Name)
return
end
end
For me, I add a space after every end statement that isn’t above another end statement. I also add spaces between returns and the previous statement. Something that isn’t here is I always put spaces between wait() functions, so it’s easier to see where the script pauses. The goal is not to have a space between every line, but to have enough spaces it improves the readability.
Operator spacing
Operator spacing is when you use spaces on operators such as *, +, -, /, and more. First off, when you declare a variable, you should space the = (equal sign) from the variable. This also goes for == (is equal to). This will look like this local x = 1
instead of local x=1
. Similarly you should do the same with ==. So we write
if x == 1 then
instead of if x==1 then
. The same thing applies to less than (<) and greater than signs (>). Now, this next one is not a must. When it comes to mathematical operators and the concatenation operator I have seen some say spacing is less readable than not. Personally I space my operators, but for this, you should choose what looks best to you. I have an example below that shows the difference.
Code
local x = 1
local y = 2
x + y
x+y
x - y
x-y
x * y
x*y
x / y
x/y
x % y
x%y
x ^ y
x^y
local string1 = "s"
local string2 = "h"
string1 .. string2
string1..string2
Functions and modules
Functions and modules are a big problem when it comes to writing clean code. This is because each function should only handle one thing. It’s so easy to get lazy and not create another function yielding the contents of three functions in one. The same thing is for modules (but not as extreme). A module should only have the functions for what it is supposed to do. If you’re making a datastore module, you should only include the functions for receiving and setting data. If you find yourself doing more than one thing per function, try moving that code into a separate function to handle that code.
Comments
Comments is one of the most simple ways to make code more readable. Don’t over use them, but use enough that if you don’t remember what your code does, you can see where certain processes occur. I am awful at this and I cannot say how many times I have come back to code and have no idea what it does. Trust me, comment, comment, comment…
Now take a look at my placement module (parts of it) with and without comments.
No comments
local function editFloor(f)
if enableFloors and not stackable then
if f == 1 then
posY = posY + floor(abs(floorStep))
else
posY = posY - floor(abs(floorStep))
end
end
end
local function calculateItemLocation()
x, z = mouse.Hit.X, mouse.Hit.Z
if moveByGrid then
if x % grid < grid / 2 then
posX = round(x - (x % grid))
else
posX = round(x + (grid - (x % grid)))
end
if z % grid < grid / 2 then
posZ = round(z - (z % grid))
else
posZ = round(z + (grid - (z % grid)))
end
if currentRot then
cx = primary.Size.X / 2
cz = primary.Size.Z / 2
else
cx = primary.Size.Z / 2
cz = primary.Size.X / 2
end
else
posX = x
posZ = z
end
if stackable and mouse.Target then
posY = calculateYPos(mouse.Target.Position.Y, mouse.Target.Size.Y, primary.Size.Y)
end
posY = clamp(posY, initialY, maxHeight + initialY)
bounds()
end
local function getFinalCFrame()
return cframe(posX, posY, posZ) * cframe(cx, 0, cz) * anglesXYZ(0, rot * pi / 180, 0)
end
local function translateObj()
if currentState ~= 4 then
calculateItemLocation()
checkHitbox()
editHitboxColor()
object:SetPrimaryPartCFrame(primary.CFrame:Lerp(cframe(posX, posY, posZ) * cframe(cx, 0, cz) * anglesXYZ(0, rot * pi / 180, 0), speed))
end
end
local function coolDown(plr, cd)
if lastPlacement[plr.UserId] == nil then
lastPlacement[plr.UserId] = os.time()
return true
else
if os.time() - lastPlacement[plr.UserId] >= cd then
lastPlacement[plr.UserId] = os.time()
return true
else
return false
end
end
end
function placement:terminate()
stackable = nil
canPlace = nil
smartRot = nil
object:Destroy()
object = nil
if displayGridTexture then
for i, v in next, plot:GetChildren() do
if v then
if v.Name == "GridTexture" and v:IsA("Texture") then
v:Destroy()
end
end
end
end
setCurrentState(4)
canActivate = true
return
end
function placement:requestPlacement(func)
if currentState ~= 4 or currentState ~= 3 and object then
local cf
calculateItemLocation()
if coolDown(player, placementCooldown) then
if buildModePlacement then
cf = getFinalCFrame()
checkHitbox()
setCurrentState(2)
if currentState == 2 then
func:InvokeServer(object.Name, placedObjects, loc, cf, collisions)
setCurrentState(1)
end
else
cf = getFinalCFrame()
checkHitbox()
setCurrentState(2)
if currentState == 2 then
if func:InvokeServer(object.Name, placedObjects, loc, cf, collisions) then
placement:terminate()
end
end
end
end
end
end
function placement:activate(id, pobj, plt, stk, r)
if object then
object:Destroy()
object = nil
end
plot = plt
object = itemLocation:FindFirstChild(tostring(id))
placedObjects = pobj
loc = itemLocation
approveActivation()
object = itemLocation:FindFirstChild(id):Clone()
for i, o in next, object:GetDescendants() do
if o then
if o:IsA("Part") or o:IsA("UnionOperation") or o:IsA("MeshPart") then
o.CanCollide = false
if transparentModel then
o.Transparency = o.Transparency + transparencyDelta
end
end
end
end
object.PrimaryPart.Transparency = hitboxTransparency
stackable = stk
smartRot = r
if not stk then
mouse.TargetFilter = placedObjects
else
mouse.TargetFilter = object
end
if buildModePlacement then
canActivate = true
else
canActivate = false
end
initialY = calculateYPos(plt.Position.Y, plt.Size.Y, object.PrimaryPart.Size.Y)
posY = initialY
speed = 0
rot = 0
currentRot = true
translateObj()
displayGrid()
editHitboxColor()
if interpolation then
speed = clamp(abs(tonumber(1 - lerpSpeed)), 0, 0.9)
else
speed = 1
end
primary = object.PrimaryPart
object.Parent = pobj
setCurrentState(1)
end
runService:BindToRenderStep("Input", Enum.RenderPriority.Input.Value, translateObj)
userInputService.InputBegan:Connect(getInput)
return placement
Commented
local placement = {}
placement.__index = placement
-- switches the floor depending on the value given
local function editFloor(f)
if enableFloors and not stackable then
if f == 1 then
posY = posY + floor(abs(floorStep))
else
posY = posY - floor(abs(floorStep))
end
end
end
-- Calculates the Y position to be ontop of the plot (all objects) and any object (when stacking)
local function calculateYPos(tp, ts, o)
return (tp + ts / 2) + o / 2
end
-- Calculates the position of the object
local function calculateItemLocation()
x, z = mouse.Hit.X, mouse.Hit.Z
if moveByGrid then
-- Snaps models to grid
if x % grid < grid / 2 then
posX = round(x - (x % grid))
else
posX = round(x + (grid - (x % grid)))
end
if z % grid < grid / 2 then
posZ = round(z - (z % grid))
else
posZ = round(z + (grid - (z % grid)))
end
-- Handles giving proper offset when rotated
if currentRot then
cx = primary.Size.X / 2
cz = primary.Size.Z / 2
else
cx = primary.Size.Z / 2
cz = primary.Size.X / 2
end
else
posX = x
posZ = z
end
-- Changes posY depending on mouse target
if stackable and mouse.Target then
posY = calculateYPos(mouse.Target.Position.Y, mouse.Target.Size.Y, primary.Size.Y)
end
-- Clamps posY to a max height above the plot position
posY = clamp(posY, initialY, maxHeight + initialY)
-- Invokes bounds
bounds()
end
--[[
Used for sending a final CFrame to the server when using interpolation.
When interpolating the position is changing. This is the position the object will
end up after the lerp is finished.
]]
local function getFinalCFrame()
return cframe(posX, posY, posZ) * cframe(cx, 0, cz) * anglesXYZ(0, rot * pi / 180, 0)
end
-- Sets the position of the object
local function translateObj()
if currentState ~= 4 then
calculateItemLocation()
checkHitbox()
editHitboxColor()
object:SetPrimaryPartCFrame(primary.CFrame:Lerp(cframe(posX, posY, posZ) * cframe(cx, 0, cz) * anglesXYZ(0, rot * pi / 180, 0), speed))
end
end
-- Makes sure that you cannot place objects too fast.
local function coolDown(plr, cd)
if lastPlacement[plr.UserId] == nil then
lastPlacement[plr.UserId] = os.time()
return true
else
if os.time() - lastPlacement[plr.UserId] >= cd then
lastPlacement[plr.UserId] = os.time()
return true
else
return false
end
end
end
-- Verifys that the plane which the object is going to be placed upon is the correct size
local function verifyPlane()
if plot.Size.X % grid == 0 and plot.Size.Z % grid == 0 then
return true
else
return false
end
end
-- Checks if there are any problems with the users setup
local function approveActivation()
if not verifyPlane() then
warn("The object that the model is moving on is not scaled correctly. Consider changing it.")
end
if grid > min(plot.Size.X, plot.Size.Z) then
error("Grid size is larger than the plot size. To fix this, try lowering the grid size.")
end
end
-- Constructor function
function placement.new(g, objs, r, t, u, l)
local data = {}
local metaData = setmetatable(data, placement)
-- Sets variables needed
grid = abs(tonumber(g))
itemLocation = objs
rotateKey = r
terminateKey = t
raiseKey = u
lowerKey = l
data.gridsize = grid
data.items = objs
data.rotate = rotateKey
data.cancel = terminateKey
data.raise = raiseKey
data.lower = lowerKey
return data
end
-- Terminates placement
function placement:terminate()
-- resets variables
stackable = nil
canPlace = nil
smartRot = nil
object:Destroy()
object = nil
-- removes grid texture from plot
if displayGridTexture then
for i, v in next, plot:GetChildren() do
if v then
if v.Name == "GridTexture" and v:IsA("Texture") then
v:Destroy()
end
end
end
end
setCurrentState(4)
canActivate = true
return
end
-- Requests to place down the object
function placement:requestPlacement(func)
if currentState ~= 4 or currentState ~= 3 and object then
local cf
calculateItemLocation()
-- Makes sure you have waited the cooldown period before placing
if coolDown(player, placementCooldown) then
-- Buildmode placement is when you can place multiple objects in one session
if buildModePlacement then
cf = getFinalCFrame()
checkHitbox()
setCurrentState(2)
-- Sends information to the server, so the object can be placed
if currentState == 2 then
func:InvokeServer(object.Name, placedObjects, loc, cf, collisions)
setCurrentState(1)
end
else
cf = getFinalCFrame()
checkHitbox()
setCurrentState(2)
if currentState == 2 then
-- Same as above (line 464)
if func:InvokeServer(object.Name, placedObjects, loc, cf, collisions) then
placement:terminate()
end
end
end
end
end
end
-- Activates placement
function placement:activate(id, pobj, plt, stk, r)
if object then
object:Destroy()
object = nil
end
-- Sets necessary variables for placement
plot = plt
object = itemLocation:FindFirstChild(tostring(id))
placedObjects = pobj
loc = itemLocation
approveActivation()
object = itemLocation:FindFirstChild(id):Clone()
-- Sets properties of the model (CanCollide, Transparency)
for i, o in next, object:GetDescendants() do
if o then
if o:IsA("Part") or o:IsA("UnionOperation") or o:IsA("MeshPart") then
o.CanCollide = false
if transparentModel then
o.Transparency = o.Transparency + transparencyDelta
end
end
end
end
object.PrimaryPart.Transparency = hitboxTransparency
stackable = stk
smartRot = r
-- Allows stackable objects depending on stk variable given by the user
if not stk then
mouse.TargetFilter = placedObjects
else
mouse.TargetFilter = object
end
-- Toggles buildmode placement (infinite placement) depending on if set true by the user
if buildModePlacement then
canActivate = true
else
canActivate = false
end
-- Gets the initial y pos and gives it to posY
initialY = calculateYPos(plt.Position.Y, plt.Size.Y, object.PrimaryPart.Size.Y)
posY = initialY
speed = 0
rot = 0
currentRot = true
translateObj()
displayGrid()
editHitboxColor()
-- Sets up interpolation speed
if interpolation then
speed = clamp(abs(tonumber(1 - lerpSpeed)), 0, 0.9)
else
speed = 1
end
-- Parents the object to the location given
primary = object.PrimaryPart
object.Parent = pobj
setCurrentState(1)
end
-- Just basic invokes
runService:BindToRenderStep("Input", Enum.RenderPriority.Input.Value, translateObj)
userInputService.InputBegan:Connect(getInput)
return placement
Conclusion
Now just because I say one thing is better than another doesn’t mean it is. If you disagree with something in this tutorial, just change it for what fits your code style. You can create your own style that is easy to read for you. Really, the only thing that is crucial, is naming functions and variables properly.
Hope you enjoyed this tutorial! Comment your thoughts on it to give feedback on it!