How should I handle reocurring events

Help and Feedback Code Review
#1

I have this terrible looking code for shopping npcs and was wondering how could I improve it.

-Should a while loop not be used?

-Should I organize variables

-Should I not use loops or should I not have everything nested in a loop.

 local CustomerService = coroutine.wrap(function()
				while task.wait(1) do
				
			for _, i in pairs(profile.Data.Plot) do
						for _, folders in pairs(PlotBuild:GetDescendants()) do
							if folders.Name == "CanvasObjects" and folders:IsA("Folder") then
							for _, buildings in pairs(folders:GetChildren()) do
								
							
						if buildings then
							if buildings.Name == i then
								if itemTypes[i] == "Stands" then
										local Stand = buildings
										if Stand and Stand.PrimaryPart then
local CustomerSpawn = Plot:FindFirstChild("CustomerSpawn")
											if CustomerSpawn:FindFirstChild("Occupied").Value == false then
												local worker = Stand.PrimaryPart:FindFirstChild("Worker")
												print(worker)
											
												local hasCustomer = worker:FindFirstChild("HasCustomer")
												
						if hasCustomer.Value == false then
							hasCustomer.Value = true
							local Customer = Npcs.customer:GetChildren()[math.random(1,#Npcs.customer:GetChildren())]:Clone()
							Customer.Parent = Stand
										
									if Customer and Stand then
											Customer:PivotTo(CFrame.new(CustomerSpawn.Position))
											CustomerSpawn:FindFirstChild("Occupied").Value = true
											CustomerMOVE(Customer,Stand,plr,Plot)
												task.wait(profile.Data.CustomerRate)
											CustomerSpawn:FindFirstChild("Occupied").Value = false
1 Like
#2

Here are some suggestions to improve the code:

  • Organize variables: Group related variables together and give them descriptive names to make the code more readable and easier to understand.
  • Avoid deeply nested loops: Deeply nested loops can make the code harder to read and understand. Consider breaking the code into smaller functions to reduce the level of nesting.
  • Use while loops judiciously: While loops are useful when you need to repeat a set of actions until a certain condition is met. However, they can also lead to infinite loops if the condition is never met. Make sure that the condition in the while loop will eventually be met and that there is a way to break out of the loop.
  • Use comments: Adding comments to your code can help others (and yourself) understand what the code is doing and why it is doing it.
#3

A few things to add on top:

  1. Never nest: It’s recommended to avoid nesting code more than three layers deep. This practice can help improve code readability and maintainability.
  2. Guard Clauses: Reverse if statement and use the return keyword to do an early exit out of a function instead of nesting your code inside an if statement
  3. Extraction: When encountering a code with too much nesting, consider taking a part of it into it’s own function
  4. Refer to the lua style guide: Roblox Lua Style guide
Example

Instead of

local function CALCULATE(b, t)
    if top > bottom then
        local Sum = 0
        for num = b, t do
            if Sum  % 2 == 0 then
                Sum  += num
            end
        end
        return Sum 
    else
        return 0
    end
end

do:

local function filterNumber(number) : number
    if number % 2 == 0 then
        return number
    end

    return 0
end

local function calculate(bottom, top) : number
    if top < bottom then
        return 0
    end

    local sum = 0
    for number = bottom, top do
        sum += filterNumber(number) 
    end
    return sum
end
1 Like